Skip to content

Commit bf42e76

Browse files
os-zhuangclaude
andauthored
fix(objectql): a nested plugin registers jobs / emailTemplates / tools / skills — the two registration copies become one enumeration (#7049) (#7153)
* fix(objectql): register jobs / emailTemplates / tools / skills from a nested plugin too (#7049) `engine.ts` reaches the ADR-0010 provenance-stamping seam (`registerItem` -> `applyProtection`) from two entry points -- a manifest and a nested plugin -- and each carried its OWN copy of the collection list. The copies had drifted by four collections: `jobs`, `emailTemplates`, `tools` and `skills` registered from a manifest and NOT from a nested plugin, so a package shipping any of them via `manifest.plugins[]` registered nothing and stamped no provenance -- no refusal, no diagnostic. Hand-adding the four names was the available alternative and is exactly what #5870 did for `capabilities`; it is what left these four undiffed. Measured the two loops against each other instead: they differ in which object they read, which package id they stamp (both resolve to the same parent package), a per-key `debug` line, and the manifest seam's aggregated-view expansion plus its warn-on-nameless-item. All four are loop-BODY differences; none is a reason for the seams to enumerate different collections. So the enumeration is hoisted to one module-scope `METADATA_ARRAY_KEYS` both seams read, and the divergence is unrepresentable rather than merely unnoticed. `check:stack-collection-maps` now pins one ObjectQL enumeration instead of two, and the waiver row recording the divergence is removed in the same change -- #6242's ratchet handshake: 8 enumerations / 19 waiver rows -> 7 / 16. Tests pin each of the four registering AND carrying `_packageId` / `_provenance` from a nested plugin, plus the seams' agreement as a property over every comparable collection rather than over the four that happened to diverge. Refs: #7049, #7032, #6242, #5870, #4509, ADR-0010. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CXFj4BYpeUHyTX4eqirbeQ * docs(objectql): name the filed card for the views-expansion seam difference (#7163) The `views` exclusion in the seam-agreement sweep is out of scope for #7049 by measurement, not by convenience: one aggregated container registers ['account', 'account.all_accounts', 'account.form'] from a manifest and ['account'] from a nested plugin. Filed as #7163; recording the issue number and the measured values so the exclusion is falsifiable from the test file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CXFj4BYpeUHyTX4eqirbeQ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent aeadbd6 commit bf42e76

5 files changed

Lines changed: 388 additions & 105 deletions

File tree

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
---
2+
"@objectstack/objectql": patch
3+
---
4+
5+
fix(objectql): a nested plugin registers `jobs` / `emailTemplates` / `tools` / `skills` — four collections it silently dropped
6+
7+
**This changes boot behaviour for packages that already ship today.** A package
8+
whose artifacts arrive through a nested plugin (`manifest.plugins[]`) and that
9+
declares any of `jobs`, `emailTemplates`, `tools` or `skills` previously
10+
registered **nothing** for those collections: no refusal, no diagnostic, no
11+
ADR-0010 provenance stamp. After this change the same package registers them,
12+
stamped to the parent package — so `/meta/job`, `/meta/email_template`,
13+
`/meta/tool` and `/meta/skill` begin answering for it, the email plugin's
14+
`sys_email_template` materializer (#4509) begins seeing its templates, and the
15+
AI protocol begins resolving its tools and skills. Anything that has been
16+
compensating for the silence — a duplicate declaration hoisted to the top-level
17+
manifest, a hand-seeded `sys_email_template` row — will now find the collection
18+
already registered.
19+
20+
`engine.ts` reaches the provenance-stamping seam (`registerItem`
21+
`applyProtection`, the only place `_packageId` / `_provenance` are written) from
22+
two entry points, and each carried its **own copy** of the collection list. The
23+
copies had drifted by exactly those four. `capabilities` hit the same divergence
24+
and was patched into the second copy by hand (#5870) without the rest of the two
25+
lists being diffed, which is how these four survived it.
26+
27+
So the copies are gone rather than reconciled: both entry points now read one
28+
module-scope `METADATA_ARRAY_KEYS`. The two loops were measured against each
29+
other first — they differ in which object they read, which package id they stamp
30+
(both resolve to the same parent package), a per-key `debug` line, and the
31+
manifest seam's aggregated-view expansion and warn-on-nameless-item. Every one
32+
of those is a loop-body difference; none is a reason for the two seams to
33+
enumerate different collections. `check:stack-collection-maps` correspondingly
34+
pins one ObjectQL enumeration instead of two, and its waiver row recording the
35+
divergence is removed with the divergence (#6242's ratchet handshake).
36+
37+
Refs: #7049, #6242, #5870, #4509, ADR-0010.

packages/objectql/src/engine-capability-provenance.test.ts

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,18 @@
88
* path: `ObjectQL.registerApp()` decomposes a manifest's metadata arrays and
99
* calls `SchemaRegistry.registerItem(type, item, 'name', packageId)`, which runs
1010
* `applyProtection(item, { packageId })` and stamps `_packageId` /
11-
* `_provenance`. The key list that drives that decomposition
12-
* (`metadataArrayKeys`, twice in `engine.ts`: the manifest seam and the nested
13-
* `registerPlugin` seam) carried every other Security-Protocol collection —
14-
* `permissions`, `sharingRules`, `roles`, `profiles`, `policies` — but not
15-
* `capabilities`.
11+
* `_provenance`. The key list that drives that decomposition — at the time,
12+
* `metadataArrayKeys`, declared TWICE in `engine.ts`, once per seam: the
13+
* manifest seam and the nested `registerPlugin` seam — carried every other
14+
* Security-Protocol collection — `permissions`, `sharingRules`, `roles`,
15+
* `profiles`, `policies` — but not `capabilities`.
16+
*
17+
* The two copies are one `METADATA_ARRAY_KEYS` since #7049, which found four
18+
* MORE collections (`jobs`, `emailTemplates`, `tools`, `skills`) that this
19+
* fix's one-name-at-a-time shape had left diverged. The seams' agreement is
20+
* pinned as a property in `engine-nested-plugin-collections.test.ts`; the
21+
* nested-seam case below stays because `capabilities` is the collection whose
22+
* absence had a NAMED downstream consequence.
1623
*
1724
* Consequence before the fix: `plugin-security`'s `bootstrapDeclaredCapabilities`
1825
* resolves the owner as `cap._packageId ?? cap.packageId` and reads its input
@@ -110,10 +117,13 @@ describe('registerApp — declared capabilities carry registry provenance (#5870
110117
});
111118

112119
it('stamps capabilities declared by a NESTED plugin too (the second seam)', () => {
113-
// `engine.ts` carries `metadataArrayKeys` twice — the manifest seam and the
114-
// `registerPlugin` seam reached via `manifest.plugins[]`. A fix applied to
115-
// only one leaves a package's nested plugin declaring capabilities that
116-
// still never get stamped, which is the same defect one level down.
120+
// `engine.ts` reaches this seam from two entry points — the manifest seam
121+
// and the `registerPlugin` seam reached via `manifest.plugins[]`. A fix
122+
// applied to only one leaves a package's nested plugin declaring
123+
// capabilities that still never get stamped, which is the same defect one
124+
// level down. (Since #7049 both entry points read ONE collection list, so
125+
// this can no longer be half-fixed; the assertion stays as the regression
126+
// pin for the collection that first exposed it.)
117127
const engine = new ObjectQL();
118128
engine.registerApp({
119129
id: PKG,
Lines changed: 222 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#7049] A nested plugin registers the SAME collections a manifest does.
5+
*
6+
* `engine.ts` reaches the ADR-0010 provenance seam — `registerItem` →
7+
* `applyProtection`, the only place `_packageId` / `_provenance` are stamped —
8+
* from two entry points: a manifest (`registerApp()`) and a nested plugin
9+
* (`registerPlugin()`, reached via `manifest.plugins[]`). Each used to carry its
10+
* OWN copy of the collection list, and the two copies had drifted:
11+
*
12+
* `jobs`, `emailTemplates`, `tools`, `skills` — registered from a manifest,
13+
* NOT registered from a nested plugin.
14+
*
15+
* A package shipping any of the four from a nested plugin therefore registered
16+
* NOTHING: no refusal, no diagnostic, no provenance stamp. The collection simply
17+
* was not in the registry after boot, and every reader of it — `/meta/job`,
18+
* `/meta/email_template`, the AI protocol's tool/skill resolution, the email
19+
* plugin's `sys_email_template` materializer (#4509) — answered empty for a
20+
* package that had declared it.
21+
*
22+
* `capabilities` hit this exact divergence and was patched into the nested copy
23+
* BY HAND (#5870); nobody then diffed the rest of the two lists, which is how
24+
* the remaining four survived. So the fix is not four more names: the two copies
25+
* are now ONE `METADATA_ARRAY_KEYS` both seams read, and the last suite below
26+
* pins that property directly — a future divergence has to get past a test that
27+
* compares the two seams' output rather than past a reviewer diffing two lists.
28+
*
29+
* Refs: #7049, #6242 (the enumeration sweep), #5870 (the `capabilities`
30+
* precedent), PR #7032 (the measurement + the gate waiver row this removes),
31+
* ADR-0010 (provenance envelope).
32+
*/
33+
34+
import { describe, it, expect } from 'vitest';
35+
import { pluralToSingular } from '@objectstack/spec/shared';
36+
import { ObjectQL } from './engine';
37+
38+
const PKG = 'com.acme.billing';
39+
40+
/**
41+
* The four collections whose two-copy divergence this card closes, each with the
42+
* singular registry type its readers ask for. Both halves matter: membership in
43+
* the shared key list registers the item, and the plural→singular mapping is
44+
* what decides whether it lands in the store anything reads (the same pairing
45+
* `engine-capability-provenance.test.ts` pins for the security collections).
46+
*/
47+
const DIVERGED_COLLECTIONS: ReadonlyArray<readonly [plural: string, singular: string]> = [
48+
['jobs', 'job'],
49+
['emailTemplates', 'email_template'],
50+
['tools', 'tool'],
51+
['skills', 'skill'],
52+
];
53+
54+
/** One authored item per collection, with NO author-side package attribution. */
55+
function itemFor(plural: string): Record<string, unknown> {
56+
switch (plural) {
57+
case 'jobs':
58+
return { name: 'nightly_invoice', label: 'Nightly Invoice', schedule: '0 2 * * *' };
59+
case 'emailTemplates':
60+
return { name: 'invoice_ready', subject: 'Your invoice is ready', body: 'Hello {{name}}' };
61+
case 'tools':
62+
return { name: 'refund_lookup', label: 'Refund Lookup', description: 'Find a refund by id.' };
63+
case 'skills':
64+
return { name: 'dunning', label: 'Dunning', description: 'Chase overdue invoices.' };
65+
default:
66+
throw new Error(`no fixture for '${plural}'`);
67+
}
68+
}
69+
70+
/** A manifest whose artifacts arrive ONLY through a nested plugin. */
71+
function manifestWithNestedPlugin(collections: readonly string[]) {
72+
const plugin: Record<string, unknown> = { name: 'billing-nested' };
73+
for (const plural of collections) plugin[plural] = [itemFor(plural)];
74+
return { id: PKG, name: 'billing', plugins: [plugin] };
75+
}
76+
77+
/** The same artifacts declared directly on the manifest — the reference path. */
78+
function manifestDirect(collections: readonly string[]) {
79+
const manifest: Record<string, unknown> = { id: PKG, name: 'billing' };
80+
for (const plural of collections) manifest[plural] = [itemFor(plural)];
81+
return manifest;
82+
}
83+
84+
function registeredNames(engine: ObjectQL, type: string): string[] {
85+
return (engine.registry.listItems<any>(type) ?? [])
86+
.map((i: any) => i?.content ?? i)
87+
.filter(Boolean)
88+
.map((i: any) => i.name);
89+
}
90+
91+
function registeredItem(engine: ObjectQL, type: string): any {
92+
return (engine.registry.listItems<any>(type) ?? []).map((i: any) => i?.content ?? i).filter(Boolean)[0];
93+
}
94+
95+
describe('registerPlugin — the four collections a nested plugin used to drop (#7049)', () => {
96+
for (const [plural, singular] of DIVERGED_COLLECTIONS) {
97+
it(`registers \`${plural}\` shipped by a nested plugin under '${singular}'`, () => {
98+
const engine = new ObjectQL();
99+
engine.registerApp(manifestWithNestedPlugin([plural]));
100+
101+
// Before #7049 this list was EMPTY — the silent no-registration.
102+
expect(registeredNames(engine, singular)).toEqual([itemFor(plural).name]);
103+
});
104+
105+
it(`stamps ADR-0010 provenance on a nested-plugin \`${plural}\` item`, () => {
106+
const engine = new ObjectQL();
107+
engine.registerApp(manifestWithNestedPlugin([plural]));
108+
109+
const item = registeredItem(engine, singular);
110+
expect(item, `nothing registered under '${singular}'`).toBeDefined();
111+
// A nested plugin contributes UNDER its parent package's ownership: the
112+
// parent already claimed the namespace, so `ownerId` is the parent id.
113+
expect(item._packageId, `'${singular}' reached the registry unstamped`).toBe(PKG);
114+
expect(item._provenance).toBe('package');
115+
// …and the stamp really is the registry's: nothing authored it.
116+
expect(item.packageId).toBeUndefined();
117+
});
118+
}
119+
120+
it('registers all four at once, exactly as a manifest does', () => {
121+
const plurals = DIVERGED_COLLECTIONS.map(([p]) => p);
122+
123+
const nested = new ObjectQL();
124+
nested.registerApp(manifestWithNestedPlugin(plurals));
125+
const direct = new ObjectQL();
126+
direct.registerApp(manifestDirect(plurals));
127+
128+
for (const [plural, singular] of DIVERGED_COLLECTIONS) {
129+
expect(registeredNames(nested, singular), `'${plural}' missing from the nested seam`)
130+
.toEqual(registeredNames(direct, singular));
131+
expect(registeredItem(nested, singular)._packageId)
132+
.toBe(registeredItem(direct, singular)._packageId);
133+
}
134+
});
135+
136+
it('maps each collection to the singular type its readers ask for', () => {
137+
// Membership in the key list is only half the contract: an item registered
138+
// under the wrong singular lands in a store nobody reads.
139+
for (const [plural, singular] of DIVERGED_COLLECTIONS) {
140+
expect(pluralToSingular(plural), `${plural} must register as '${singular}'`).toBe(singular);
141+
}
142+
});
143+
});
144+
145+
describe('the two registration seams enumerate ONE collection list (#7049)', () => {
146+
/**
147+
* The root cause this card names is not the four missing names — it is that
148+
* the two seams had two hand-maintained lists and nothing compared them. This
149+
* suite is that comparison, stated as a property over EVERY collection the
150+
* engine registers rather than over the four that happened to diverge: ship
151+
* one item of a collection through a manifest and the same item through a
152+
* nested plugin, and the two registries must agree. A key added to one seam
153+
* only cannot pass it, whatever the two lists look like.
154+
*
155+
* The collections excluded below are excluded for measured reasons, not to
156+
* make the test pass — see each entry.
157+
*/
158+
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'],
167+
// Retired kinds the loop still iterates; the schema rejects the keys long
168+
// before either seam runs, so a fixture cannot exercise them (the gate
169+
// carries them as an `extra` waiver row for the same reason).
170+
['workflows', 'ADR-0019 retired'],
171+
['approvals', 'ADR-0020 retired'],
172+
['roles', 'ADR-0090 retired'],
173+
['profiles', 'ADR-0088 retired'],
174+
['policies', 'ADR-0088 retired'],
175+
['ragPipelines', 'not declared by ObjectStackDefinitionSchema'],
176+
];
177+
178+
const excluded = new Set(NOT_COMPARABLE.map(([k]) => k));
179+
180+
/**
181+
* Read the shared enumeration back off the engine by probing it: for each
182+
* candidate collection, does a one-item manifest register anything? This asks
183+
* the running engine rather than importing a constant, so the test measures
184+
* behaviour and not the same literal the implementation reads.
185+
*/
186+
const CANDIDATES = [
187+
'actions', 'pages', 'dashboards', 'reports', 'datasets', 'themes',
188+
'flows', 'webhooks', 'jobs',
189+
'permissions', 'capabilities', 'sharingRules',
190+
'agents', 'tools', 'skills', 'apis',
191+
'hooks', 'mappings', 'analyticsCubes', 'connectors',
192+
'emailTemplates', 'docs', 'books',
193+
].filter((k) => !excluded.has(k));
194+
195+
it.each(CANDIDATES)('registers `%s` identically from a manifest and from a nested plugin', (plural) => {
196+
const singular = pluralToSingular(plural);
197+
const item = { name: `probe_${plural}`, label: `Probe ${plural}` };
198+
199+
const direct = new ObjectQL();
200+
direct.registerApp({ id: PKG, name: 'billing', [plural]: [item] });
201+
const nested = new ObjectQL();
202+
nested.registerApp({ id: PKG, name: 'billing', plugins: [{ name: 'p', [plural]: [item] }] });
203+
204+
const fromManifest = registeredNames(direct, singular);
205+
const fromPlugin = registeredNames(nested, singular);
206+
207+
// The property, in both directions: whatever one seam registers, the other
208+
// registers. A collection added to one list only fails here.
209+
expect(fromPlugin, `'${plural}' diverges between the two registration seams`).toEqual(fromManifest);
210+
if (fromManifest.length > 0) {
211+
expect(registeredItem(nested, singular)._packageId).toBe(PKG);
212+
expect(registeredItem(nested, singular)._provenance).toBe('package');
213+
}
214+
});
215+
216+
it('records why each excluded collection is not comparable, rather than dropping it silently', () => {
217+
for (const [, why] of NOT_COMPARABLE) expect(why.length).toBeGreaterThan(0);
218+
expect(NOT_COMPARABLE.map(([k]) => k)).toEqual([
219+
'views', 'workflows', 'approvals', 'roles', 'profiles', 'policies', 'ragPipelines',
220+
]);
221+
});
222+
});

0 commit comments

Comments
 (0)