Skip to content

Commit edbf873

Browse files
os-zhuangclaude
andauthored
fix(objectql): expand an aggregated views container from the nested-plugin seam too (#7163) (#7452)
ADR-0017 makes the loader dual-read: an aggregated `defineView` container is registered under the bare `<object>` key for back-compatible reads AND expanded into independent `ViewItem`s under `<object>.<viewKey>`. Only the expanded items carry `viewKind`, and `getViewsByObject()` filters on exactly that — so the expanded layer, not the container, is what `GET /meta/view?object=`, the view switcher and Studio's package attribution read. `engine.ts` reaches the registration seam from two entry points and only the manifest one expanded. One container measured through each: via manifest → ['account', 'account.all_accounts', 'account.form'] via nested plugin → ['account'] No refusal, no diagnostic — a package shipping views through `manifest.plugins[]` had no views as far as every reader of the expanded layer was concerned. The direction was measured, not assumed. ADR-0017 states the dual-read as a property of "the loader" at load time rather than of one entry point; the other loader (`MetadataPlugin`'s artifact/HMR path) expands too, which is why the shared implementation was pushed into `@objectstack/spec` "so the two loaders cannot drift"; every authored stack in the tree ships `views` at manifest top level, so removing the manifest expansion would take the switcher from all of them; and no in-tree package ships `views` through a nested plugin, so the seam that gains behaviour breaks nobody. One direction is load-bearing, the other is not. So the copies are gone rather than reconciled: both seams now run one `registerMetadataCollections()`. #7049 hoisted the shared `METADATA_ARRAY_KEYS` and recorded that the bodies still differed in a per-key `debug` line, this expansion, and a warn-on-nameless-item. Sharing the list made the seams' collection SET unanswerable-differently; sharing the body does the same for what they DO with a collection both see — a nested plugin now also emits the nameless-item warning it used to swallow. Pins: manifest-vs-nested parity for an aggregated container, the expanded per-view identities (name, `viewKind`, object, isDefault, order, config, ADR-0010 provenance stamped to the parent package), the ADR-0017 dual-read back-compat half, and a control that a non-aggregated `ViewItem` still registers identically through both seams with no expansion. `engine-nested-plugin-collections.test.ts`'s `views` exclusion row — the only one that was ever a behaviour difference rather than a retired kind — is removed with the divergence, and `views` rejoins its cross-seam property test. Refs: #7163, #7049, #6242, #5870, ADR-0017, ADR-0010. Claude-Session: https://claude.ai/code/session_01BPVc5WY75PkeXoA5NFiEEk Co-authored-by: Claude <noreply@anthropic.com>
1 parent eea20c7 commit edbf873

4 files changed

Lines changed: 394 additions & 59 deletions

File tree

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
fix(objectql): a nested plugin expands an aggregated `views` container — its per-view items reached no registry
6+
7+
**This changes boot behaviour for packages that ship views through
8+
`manifest.plugins[]`.** "Object has-many View" (ADR-0017 §2, §3.2) makes the
9+
loader **dual-read**: an aggregated `defineView` container is registered under
10+
the bare `<object>` key for back-compatible reads AND expanded into independent
11+
`ViewItem`s under `<object>.<viewKey>`. Only the expanded items carry
12+
`viewKind`, and `getViewsByObject()` filters on exactly that — so the expanded
13+
layer, not the container, is what `GET /meta/view?object=`, the runtime view
14+
switcher and Studio's package attribution actually read.
15+
16+
`engine.ts` reaches the registration seam from two entry points, and only the
17+
manifest one expanded. One container measured through each:
18+
19+
```
20+
via manifest → ['account', 'account.all_accounts', 'account.form']
21+
via nested plugin → ['account']
22+
```
23+
24+
No refusal and no diagnostic: a package whose views arrived through a nested
25+
plugin registered the container and nothing else, so every reader of the
26+
expanded layer saw an object with no views at all. After this change both seams
27+
answer the same, and those packages' view switchers begin working. Anything that
28+
has been compensating for the silence — a duplicate `views:` hoisted to the
29+
top-level manifest — now finds the views already registered.
30+
31+
The direction was measured rather than assumed, because the divergence had two
32+
coherent readings. ADR-0017 states the dual-read as a property of "the loader"
33+
at load time, not of one entry point; the OTHER loader agrees with the manifest
34+
seam (`MetadataPlugin`'s artifact/HMR path expands too, which is why the shared
35+
implementation lives in `@objectstack/spec` — "so the two loaders cannot
36+
drift"); every authored stack in the tree ships `views` at manifest top level,
37+
so removing the manifest seam's expansion would take the switcher away from all
38+
of them; and no in-tree package ships `views` through a nested plugin, so the
39+
seam that GAINS behaviour here breaks nobody. One direction is load-bearing for
40+
real consumers and the other is not.
41+
42+
So, as with #7049, the copies are gone rather than reconciled: both seams now
43+
run one `registerMetadataCollections()` body. #7049 hoisted the shared
44+
`METADATA_ARRAY_KEYS` and measured the loop bodies on the way past, recording
45+
that they still differed in a per-key `debug` line, this view expansion, and a
46+
warn-on-nameless-item — sharing the list made "which collections does a seam
47+
see?" unanswerable-differently while leaving "what does a seam DO with a
48+
collection both see?" answered in two places. Both remaining differences had the
49+
same structure, a body copied then improved on one side only, so the body is
50+
shared too: a nested plugin now also emits the skipping-a-nameless-item warning
51+
it used to swallow. `engine-nested-plugin-collections.test.ts`'s `views`
52+
exclusion row — the only one that was ever a behaviour difference rather than a
53+
retired kind — is removed with the divergence.
54+
55+
Refs: #7163, #7049, #6242, #5870, ADR-0017, ADR-0010.

packages/objectql/src/engine-nested-plugin-collections.test.ts

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -156,14 +156,14 @@ describe('the two registration seams enumerate ONE collection list (#7049)', ()
156156
* make the test pass — see each entry.
157157
*/
158158
const NOT_COMPARABLE: ReadonlyArray<readonly [key: string, why: string]> = [
159-
// `views` has no top-level `name` and the manifest seam additionally expands
160-
// an aggregated container into per-view items (ADR-0017); that expansion is
161-
// a LOOP-BODY difference between the seams, not an enumeration difference,
162-
// and closing it changes what a nested plugin serves. Measured while closing
163-
// this card and filed as #7163 rather than folded in: one container
164-
// registers `['account', 'account.all_accounts', 'account.form']` from a
165-
// manifest and `['account']` from a nested plugin.
166-
['views', 'manifest seam additionally expands aggregated containers (ADR-0017) — #7163'],
159+
// `views` used to sit here: the manifest seam expanded an aggregated
160+
// container into per-view items (ADR-0017) and the nested seam did not — a
161+
// LOOP-BODY difference rather than an enumeration one, so this card filed it
162+
// as #7163 instead of folding it in. #7163 closed it by sharing the body
163+
// (`registerMetadataCollections()`), so `views` is comparable now and is
164+
// back in `CANDIDATES` below; the aggregated-container half of its parity is
165+
// pinned in `engine-nested-plugin-view-expansion.test.ts`.
166+
//
167167
// Retired kinds the loop still iterates; the schema rejects the keys long
168168
// before either seam runs, so a fixture cannot exercise them (the gate
169169
// carries them as an `extra` waiver row for the same reason).
@@ -184,7 +184,7 @@ describe('the two registration seams enumerate ONE collection list (#7049)', ()
184184
* behaviour and not the same literal the implementation reads.
185185
*/
186186
const CANDIDATES = [
187-
'actions', 'pages', 'dashboards', 'reports', 'datasets', 'themes',
187+
'actions', 'views', 'pages', 'dashboards', 'reports', 'datasets', 'themes',
188188
'flows', 'webhooks', 'jobs',
189189
'permissions', 'capabilities', 'sharingRules',
190190
'agents', 'tools', 'skills', 'apis',
@@ -215,8 +215,12 @@ describe('the two registration seams enumerate ONE collection list (#7049)', ()
215215

216216
it('records why each excluded collection is not comparable, rather than dropping it silently', () => {
217217
for (const [, why] of NOT_COMPARABLE) expect(why.length).toBeGreaterThan(0);
218+
// `views` left this list in #7163 — the only entry that was ever excluded
219+
// for a BEHAVIOUR difference rather than a retired-kind one. Every survivor
220+
// is a kind the schema rejects before either seam runs.
218221
expect(NOT_COMPARABLE.map(([k]) => k)).toEqual([
219-
'views', 'workflows', 'approvals', 'roles', 'profiles', 'policies', 'ragPipelines',
222+
'workflows', 'approvals', 'roles', 'profiles', 'policies', 'ragPipelines',
220223
]);
224+
expect(NOT_COMPARABLE.map(([k]) => k)).not.toContain('views');
221225
});
222226
});
Lines changed: 231 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,231 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#7163] A nested plugin EXPANDS an aggregated view container, exactly as a
5+
* manifest does.
6+
*
7+
* ## What was broken
8+
*
9+
* "Object has-many View" (ADR-0017 §2, §3.2) makes the loader **dual-read**: an
10+
* aggregated `defineView` container is registered under the bare `<object>` key
11+
* for back-compatible reads AND expanded into independent `ViewItem`s under
12+
* `<object>.<viewKey>`. Only the expanded items carry `viewKind`, and
13+
* `getViewsByObject()` filters on exactly that — so the expanded layer, not the
14+
* container, is what `GET /meta/view?object=`, the runtime view switcher and
15+
* Studio's package attribution actually read.
16+
*
17+
* `engine.ts` reaches the registration seam from two entry points. #7049 hoisted
18+
* the shared `METADATA_ARRAY_KEYS` so both seams ENUMERATE `views`, but only the
19+
* manifest seam's loop body expanded:
20+
*
21+
* via manifest → ['account', 'account.all_accounts', 'account.form']
22+
* via nested plugin → ['account']
23+
*
24+
* No refusal and no diagnostic — a package shipping its views through
25+
* `manifest.plugins[]` simply had no views as far as every reader of the
26+
* expanded layer was concerned.
27+
*
28+
* ## Why the nested seam gained the expansion, rather than the manifest seam
29+
* ## losing it
30+
*
31+
* The divergence had two coherent readings and the card deliberately picked
32+
* neither. Measured on `main` before choosing:
33+
*
34+
* - ADR-0017 §2 states the dual-read as a property of "the loader" at load
35+
* time, not of one entry point, and §3.2 spells out that it registers BOTH.
36+
* - The OTHER loader agrees with the manifest seam: `MetadataPlugin`'s artifact
37+
* /HMR path expands too (`packages/metadata/src/plugin.ts`), which is why
38+
* the shared implementation was pushed down into `@objectstack/spec` in the
39+
* first place — "so the two loaders cannot drift" (`ui/view.zod.ts`).
40+
* - Every authored stack in the tree ships `views` at manifest TOP level
41+
* (~51 files: `examples/app-crm`, `app-todo`, `app-showcase`,
42+
* `packages/qa/downstream-contract`, …). Removing the manifest seam's
43+
* expansion would take the view switcher away from all of them.
44+
* - No in-tree package ships `views` through a nested plugin — swept across
45+
* `examples/`, `apps/` and `packages/`, zero hits. So the seam that gains
46+
* behaviour here breaks nobody in-tree, while the seam that would lose it
47+
* breaks everybody.
48+
*
49+
* One direction is load-bearing for real consumers and the other is not, so the
50+
* nested seam catches up — the same direction #7049 took for the enumeration.
51+
*
52+
* Refs: #7163, #7049 (the enumeration half + the exclusion row this retires),
53+
* ADR-0017 (Object has-many View), ADR-0010 (provenance envelope).
54+
*/
55+
56+
import { describe, it, expect } from 'vitest';
57+
import { ObjectQL } from './engine';
58+
59+
const PKG = 'com.acme.sales';
60+
61+
/**
62+
* The card's measured fixture: one aggregated container for `account`, with a
63+
* named list view and a default form. Shaped like `examples/app-crm`'s view
64+
* modules — the container carries no top-level `name`, so its registry key is
65+
* resolved from `list.data.object` (ADR-0017 §3.1: `ViewSchema` has no `name`).
66+
*
67+
* The default `list` deliberately RESTATES `listViews.all_accounts` verbatim —
68+
* the common "default == the named view" authoring pattern the expander
69+
* collapses by structural signature. That is what makes the expansion exactly
70+
* the card's measured `['account', 'account.all_accounts', 'account.form']`
71+
* rather than carrying a separate `account.default`.
72+
*/
73+
function accountContainer() {
74+
const allAccounts = {
75+
label: 'All Accounts',
76+
type: 'grid',
77+
data: { provider: 'object', object: 'account' },
78+
columns: [{ field: 'name' }],
79+
};
80+
return {
81+
list: { ...allAccounts },
82+
listViews: { all_accounts: { ...allAccounts } },
83+
form: {
84+
type: 'simple',
85+
data: { provider: 'object', object: 'account' },
86+
sections: [{ label: 'Info', fields: [{ field: 'name' }] }],
87+
},
88+
};
89+
}
90+
91+
/** The same container declared directly on the manifest — the reference path. */
92+
function viaManifest() {
93+
return { id: PKG, name: 'sales', views: [accountContainer()] };
94+
}
95+
96+
/** …and arriving ONLY through a nested plugin (`manifest.plugins[]`). */
97+
function viaNestedPlugin() {
98+
return { id: PKG, name: 'sales', plugins: [{ name: 'sales-nested', views: [accountContainer()] }] };
99+
}
100+
101+
function viewItems(engine: ObjectQL): any[] {
102+
return (engine.registry.listItems<any>('view') ?? [])
103+
.map((i: any) => i?.content ?? i)
104+
.filter(Boolean);
105+
}
106+
107+
function viewNames(engine: ObjectQL): string[] {
108+
return viewItems(engine).map((v: any) => v.name).sort();
109+
}
110+
111+
function boot(manifest: unknown): ObjectQL {
112+
const engine = new ObjectQL();
113+
engine.registerApp(manifest as any);
114+
return engine;
115+
}
116+
117+
describe('aggregated view container — the two seams register the SAME thing (#7163)', () => {
118+
it('expands a nested plugin\'s container into per-view items', () => {
119+
// Before #7163 this was exactly `['account']` — the container alone.
120+
expect(viewNames(boot(viaNestedPlugin()))).toEqual([
121+
'account',
122+
'account.all_accounts',
123+
'account.form',
124+
]);
125+
});
126+
127+
it('registers identically whether the container arrives via manifest or nested plugin', () => {
128+
// The parity pin — the card's whole point. Stated as manifest-vs-nested
129+
// equality rather than against a literal, so a change to the expansion
130+
// rules moves BOTH seams or fails here.
131+
expect(viewNames(boot(viaNestedPlugin()))).toEqual(viewNames(boot(viaManifest())));
132+
});
133+
134+
it('keeps the bare <object> container registered alongside the expansion (ADR-0017 dual-read)', () => {
135+
// Back-compat half: expanding must not replace the container. Both seams.
136+
for (const manifest of [viaManifest(), viaNestedPlugin()]) {
137+
const names = viewNames(boot(manifest));
138+
expect(names).toContain('account');
139+
expect(names.filter((n) => n.startsWith('account.'))).not.toHaveLength(0);
140+
}
141+
});
142+
});
143+
144+
describe('the expanded per-view identities a nested plugin now produces (#7163)', () => {
145+
const items = viewItems(boot(viaNestedPlugin()));
146+
const byName = Object.fromEntries(items.map((v: any) => [v.name, v]));
147+
148+
it('gives every expanded item the `viewKind` `getViewsByObject()` filters on', () => {
149+
// This is the property the whole card turns on: the container carries NO
150+
// `viewKind`, so a registry holding only the container answers empty to
151+
// `getViewsByObject()` / `GET /meta/view?object=` — silently.
152+
expect(byName['account'].viewKind).toBeUndefined();
153+
expect(byName['account.all_accounts'].viewKind).toBe('list');
154+
expect(byName['account.form'].viewKind).toBe('form');
155+
156+
const readable = items.filter((v: any) => v.viewKind && v.object === 'account');
157+
expect(readable.map((v: any) => v.name).sort()).toEqual([
158+
'account.all_accounts',
159+
'account.form',
160+
]);
161+
});
162+
163+
it('binds each expanded item to its object and stamps scope=package', () => {
164+
for (const name of ['account.all_accounts', 'account.form']) {
165+
expect(byName[name].object).toBe('account');
166+
expect(byName[name].scope).toBe('package');
167+
}
168+
});
169+
170+
it('carries the container\'s config through to the expanded item', () => {
171+
expect(byName['account.all_accounts'].config.type).toBe('grid');
172+
expect(byName['account.all_accounts'].label).toBe('All Accounts');
173+
expect(byName['account.all_accounts'].config.columns).toEqual([{ field: 'name' }]);
174+
expect(byName['account.form'].config.sections[0].label).toBe('Info');
175+
// The named entry absorbed the structurally identical default `list`, so it
176+
// is the declared default of its family.
177+
expect(byName['account.all_accounts'].isDefault).toBe(true);
178+
});
179+
180+
it('stamps ADR-0010 provenance on the expanded items, owned by the PARENT package', () => {
181+
// A nested plugin contributes under its parent's ownership — the parent
182+
// already claimed the namespace (same rule #7049 pinned for the four
183+
// collections). The expansion must not bypass that stamp.
184+
for (const name of ['account', 'account.all_accounts', 'account.form']) {
185+
expect(byName[name]._packageId, `'${name}' reached the registry unstamped`).toBe(PKG);
186+
expect(byName[name]._provenance).toBe('package');
187+
}
188+
});
189+
190+
it('produces the same expanded identities the manifest seam produces', () => {
191+
const fromManifest = Object.fromEntries(
192+
viewItems(boot(viaManifest())).map((v: any) => [v.name, v]),
193+
);
194+
for (const name of ['account.all_accounts', 'account.form']) {
195+
expect(byName[name].viewKind).toBe(fromManifest[name].viewKind);
196+
expect(byName[name].object).toBe(fromManifest[name].object);
197+
expect(byName[name].isDefault).toBe(fromManifest[name].isDefault);
198+
expect(byName[name].order).toBe(fromManifest[name].order);
199+
expect(byName[name].config).toEqual(fromManifest[name].config);
200+
}
201+
});
202+
});
203+
204+
describe('control — a NON-aggregated view is unchanged by this card (#7163)', () => {
205+
/**
206+
* The fix is scoped by `isAggregatedViewContainer`, which is false for an
207+
* already-independent `ViewItem` (it carries `viewKind`). Such a view must
208+
* register exactly once, under its own name, through BOTH seams — no
209+
* expansion, no new keys. This is what says the change is additive and only
210+
* on the container shape.
211+
*/
212+
const viewItem = {
213+
name: 'account.hot',
214+
object: 'account',
215+
viewKind: 'list',
216+
config: { type: 'grid', columns: [{ field: 'name' }] },
217+
};
218+
219+
it('registers a standalone ViewItem identically from both seams, with no expansion', () => {
220+
const direct = boot({ id: PKG, name: 'sales', views: [viewItem] });
221+
const nested = boot({ id: PKG, name: 'sales', plugins: [{ name: 'p', views: [viewItem] }] });
222+
223+
expect(viewNames(nested)).toEqual(['account.hot']);
224+
expect(viewNames(nested)).toEqual(viewNames(direct));
225+
});
226+
227+
it('leaves a container-free manifest with no view items at all', () => {
228+
expect(viewNames(boot({ id: PKG, name: 'sales' }))).toEqual([]);
229+
expect(viewNames(boot({ id: PKG, name: 'sales', plugins: [{ name: 'p' }] }))).toEqual([]);
230+
});
231+
});

0 commit comments

Comments
 (0)