fix(plugins): retire the i?.content ?? i unwrap family from plugin read paths (#8378)

.changeset/retire-content-unwrap-family.md

@@ -0,0 +1,53 @@
+---
+"@objectstack/plugin-email": patch
+"@objectstack/plugin-security": patch
+"@objectstack/plugin-sharing": patch
+"@objectstack/plugin-webhooks": patch
+---
+
+fix(plugins): a declared item reaches its schema intact — retire the `i?.content ?? i` unwrap from plugin read paths (#8378)
+
+Ten production reads over `SchemaRegistry.listItems` unwrapped every declared
+item as `i?.content ?? i`, presuming a `{ name, content }` storage envelope.
+That envelope has **no producer**. Re-measured at these seams rather than
+inherited from #7519's measurement of `MetadataFacade`:
+
+- `registerMetadataCollections` (objectql) registers each stack-collection
+  element as-is — `registerItem(type, item, 'name')`, no boxing;
+- `loadMetaFromDb` registers `convertStoredItem(JSON.parse(record.metadata))` —
+  the parsed body, never the `sys_metadata` row (whose body column is
+  `metadata`, not `content`);
+- the facade's own interim boxing of non-object values, the one writer that ever
+  produced the shape, was removed by #8349.
+
+**Removal is a fix, not a cleanup.** None of the types read through these seams
+— `permission`, `position`, `capability`, `object`, `sharingRule`, `webhook`,
+`emailTemplate` — declares a stored `content` key; every one of them rejects it
+as an unrecognized key. So wherever the key did appear the unwrap replaced a
+whole authoring document with one of its values, and `''` — falsy but
+non-nullish — passed `??` and then died at the reader's own `filter(Boolean)`,
+dropping the item with no warning, no count and no row.
+
+**On email templates the harm was sharpest, and it is the one users will
+notice.** `content` really is a spelling an author can write there:
+`EmailTemplateDefinitionSchema` lists it in its `strictObject` **aliases** table
+(`content: 'bodyHtml'`). That table is a *rejection* facility, not a conversion —
+it feeds `strictUnknownKeyError`, which runs only on the `unrecognized_keys`
+path and only builds a message; nothing rewrites the key, and the ADR-0087
+conversion layer has no `email_template` entry either. The schema was therefore
+always ready with the author's fix, and the unwrap was the one thing standing
+between the author and it: the HTML string reached
+`EmailTemplateDefinitionSchema.parse()`, which answered `Invalid input: expected
+object, received string`, and the boot warning's `name` field came back
+`undefined` — so an operator could not even tell **which** template had failed.
+
+A template authored with `content` now yields what it was always meant to:
+
+> Unrecognized key(s) on this email template: `content`. Did you mean
+> `content` → `bodyHtml`?
+
+…named against the template it came from, and counted as `skipped` rather than
+vanishing.
+
+No behaviour changes for spec-valid metadata: the reads hand back exactly the
+documents they always did.

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

@@ -24,8 +24,10 @@
  * Consequence before the fix: `plugin-security`'s `bootstrapDeclaredCapabilities`
  * resolves the owner as `cap._packageId ?? cap.packageId` and reads its input
  * with `readDeclared(ql, 'capability')`, which is
- * `engine.registry.listItems('capability')` mapped through `i?.content ?? i`
- * (`packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts:61-69`).
+ * `engine.registry.listItems('capability')`, filtered
+ * (`packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts`).
+ * That read used to map through `i?.content ?? i`; #8378 retired the unwrap
+ * across this family after measuring the envelope has no producer.
  * With `capabilities` outside the seam that list was ALWAYS empty, so the
  * `_packageId` half of that `??` could never be satisfied and the author-side
  * `packageId` — documented in `CapabilityDeclarationSchema` as the *fallback*
@@ -48,11 +50,15 @@ import { ObjectQL } from './engine';
 /**
  * The exact read `bootstrapDeclaredCapabilities` performs on the engine, kept
  * in one place so the shape this suite depends on is stated once:
- * `readDeclared(ql, type)` → `registry.listItems(type)` unwrapped by
- * `content ?? item`.
+ * `readDeclared(ql, type)` → `registry.listItems(type)`, filtered.
+ *
+ * [#8378] The `content ?? item` unwrap this helper used to mirror is gone from
+ * the production read. It was a no-op here — these tests drive a REAL
+ * `ObjectQL`, whose `registerMetadataCollections` registers each item as-is —
+ * which is exactly the measurement that retired it.
  */
 function readDeclaredShape(engine: ObjectQL, type: string): any[] {
-  return (engine.registry.listItems<any>(type) ?? []).map((i: any) => i?.content ?? i).filter(Boolean);
+  return (engine.registry.listItems<any>(type) ?? []).filter(Boolean);
 }
 
 const PKG = 'com.acme.exporter';

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

@@ -81,15 +81,17 @@ function manifestDirect(collections: readonly string[]) {
   return manifest;
 }
 
+// [#8378] Both helpers mirrored the plugin readers' `content ?? item` unwrap,
+// which has been retired: `registerMetadataCollections` registers each
+// collection element as-is, so the unwrap was a no-op on this real engine.
 function registeredNames(engine: ObjectQL, type: string): string[] {
   return (engine.registry.listItems<any>(type) ?? [])
-    .map((i: any) => i?.content ?? i)
     .filter(Boolean)
     .map((i: any) => i.name);
 }
 
 function registeredItem(engine: ObjectQL, type: string): any {
-  return (engine.registry.listItems<any>(type) ?? []).map((i: any) => i?.content ?? i).filter(Boolean)[0];
+  return (engine.registry.listItems<any>(type) ?? []).filter(Boolean)[0];
 }
 
 describe('registerPlugin — the four collections a nested plugin used to drop (#7049)', () => {

packages/objectql/src/engine-nested-plugin-view-expansion.test.ts

@@ -98,9 +98,10 @@ function viaNestedPlugin() {
   return { id: PKG, name: 'sales', plugins: [{ name: 'sales-nested', views: [accountContainer()] }] };
 }
 
+// [#8378] The `content ?? item` unwrap this mirrored is retired — a no-op on a
+// real engine, whose `registerMetadataCollections` registers items as-is.
 function viewItems(engine: ObjectQL): any[] {
   return (engine.registry.listItems<any>('view') ?? [])
-    .map((i: any) => i?.content ?? i)
     .filter(Boolean);
 }
 

packages/plugins/plugin-email/src/bootstrap-declared-email-templates.test.ts

@@ -39,9 +39,15 @@ class FakeEngine {
     if (seed?.declared) this.declared = JSON.parse(JSON.stringify(seed.declared));
   }
 
+  // [#8378] Registers items EXACTLY as the real engine does — the document
+  // itself. This fake used to box each one as `{ content: <item> }`, which made
+  // it the only producer of that envelope anywhere in the tree: a fiction that
+  // kept the production `i?.content ?? i` looking load-bearing while nothing
+  // real ever wrote the shape (`registerMetadataCollections` registers items
+  // as-is; `loadMetaFromDb` registers the parsed body, not the row).
   get _registry() {
     return {
-      listItems: (type: string) => (this.declared[type] ?? []).map((content) => ({ content })),
+      listItems: (type: string) => [...(this.declared[type] ?? [])],
     };
   }
 
@@ -261,7 +267,9 @@ describe('bootstrapDeclaredEmailTemplates', () => {
 
   it('falls back to the metadata service when the registry is empty', async () => {
     const engine = new FakeEngine();
-    const metadataService = { list: () => [{ content: declaredTemplate() }] };
+    // [#8378] `IMetadataService.list` answers the documents themselves; the
+    // `{ content: … }` box this fixture used to build had no producer.
+    const metadataService = { list: () => [declaredTemplate()] };
 
     const result = await bootstrapDeclaredEmailTemplates(engine as any, metadataService);
 
@@ -348,3 +356,77 @@ describe('mapTemplateToRow', () => {
     expect(row).not.toHaveProperty('variables_json');
   });
 });
+
+// ---------------------------------------------------------------------------
+// [#8378] The retired `i?.content ?? i` unwrap
+// ---------------------------------------------------------------------------
+
+/**
+ * `content` is a spelling an author really can write on an email template —
+ * `EmailTemplateDefinitionSchema` lists it in its `strictObject` **aliases**
+ * table (`content: 'bodyHtml'`). That table is a REJECTION facility, not a
+ * conversion: it feeds `strictUnknownKeyError`, which runs only on the
+ * `unrecognized_keys` path and only builds a *message*. Nothing rewrites the
+ * key — the ADR-0087 conversion registry has zero `email_template` entries, so
+ * `normalizeStackInput` emits no notice and leaves `content` where it was
+ * written.
+ *
+ * So when the key reaches this bridge (a `defineStack(…, { strict: false })`
+ * load, a hand-built manifest, a direct registry write — every validating door
+ * refuses it first), the schema is ready with the author's fix. The unwrap was
+ * the one thing standing between the author and their own prescription: it
+ * replaced the document with the HTML string, and a string cannot carry a
+ * key-level rejection.
+ *
+ * Both cases below put a `content` key genuinely IN PLAY — the point of the
+ * fixture. A template that never spells `content` exercises the unwrap's
+ * `?? i` arm only, and would pass against a completely unfixed tree.
+ */
+describe('declared email templates carrying the `content` alias spelling (#8378)', () => {
+  it('reaches the schema as a DOCUMENT, so the rejection carries the `content` → `bodyHtml` fix', async () => {
+    const engine = new FakeEngine({
+      declared: {
+        email_template: [
+          // `bodyHtml` deliberately absent — `content` is the author's attempt
+          // at it, which is exactly what the alias table exists to answer.
+          { name: 'auth.welcome', label: 'Welcome', subject: 'Hi', content: '<h1>Welcome</h1>' },
+        ],
+      },
+    });
+    const warn = vi.fn();
+
+    const result = await bootstrapDeclaredEmailTemplates(engine as any, undefined, { warn });
+
+    expect(result).toEqual({ seeded: 0, skipped: 1 });
+    expect(rowsOf(engine)).toHaveLength(0);
+    expect(warn).toHaveBeenCalledTimes(1);
+
+    const [, meta] = warn.mock.calls[0];
+    // The unwrap used to hand `.parse()` a bare string, so the diagnostic could
+    // name neither the offending key nor the template it came from.
+    expect(meta.name).toBe('auth.welcome');
+    expect(meta.error).toContain('content');
+    expect(meta.error).toContain('bodyHtml');
+    expect(meta.error).not.toContain('expected object, received string');
+  });
+
+  it('does not silently vanish when `content` is the empty string', async () => {
+    const engine = new FakeEngine({
+      declared: {
+        email_template: [
+          // `''` is falsy but NON-nullish, so it passed `??` and was then
+          // dropped by the reader's own `filter(Boolean)` — the template
+          // disappeared with no warning, no count and no row.
+          { name: 'ops.digest', label: 'Digest', subject: 'Daily', content: '' },
+        ],
+      },
+    });
+    const warn = vi.fn();
+
+    const result = await bootstrapDeclaredEmailTemplates(engine as any, undefined, { warn });
+
+    expect(result).toEqual({ seeded: 0, skipped: 1 });
+    expect(warn).toHaveBeenCalledTimes(1);
+    expect(warn.mock.calls[0][1].name).toBe('ops.digest');
+  });
+});

packages/plugins/plugin-email/src/bootstrap-declared-email-templates.ts

@@ -104,14 +104,57 @@ function uid(prefix: string): string {
 /**
  * Read declared `email_template` items from the ObjectQL registry (where the
  * manifest decomposition parks `stack.emailTemplates`), falling back to the
- * metadata service. Items may be wrapped as `{ content }` — unwrap to the raw
- * authoring object.
+ * metadata service. Both reads hand back the authoring document itself.
+ *
+ * ## [#8378] Why there is no `i?.content ?? i` here any more
+ *
+ * The sentence this docblock used to end with — "Items may be wrapped as
+ * `{ content }` — unwrap to the raw authoring object" — described an envelope
+ * with **no producer**. Re-measured at this seam rather than inherited from
+ * #7519's measurement of `MetadataFacade`:
+ *
+ *  - `registerMetadataCollections` (objectql `engine.ts`) registers each
+ *    `stack.emailTemplates` element as-is — `registerItem(type, item, 'name')`,
+ *    no boxing;
+ *  - `loadMetaFromDb` (metadata-protocol) registers
+ *    `convertStoredItem(JSON.parse(record.metadata))` — the parsed body, never
+ *    the `sys_metadata` row (whose body column is `metadata`, not `content`);
+ *  - `MetadataFacade`'s own interim boxing of non-object values, the one writer
+ *    that ever produced the shape, was removed by #8349.
+ *
+ * ## …and why removing it is a FIX rather than a tidy-up
+ *
+ * `content` IS a spelling an author can write on an email template — but as a
+ * **rejection alias**, not a conversion. `EmailTemplateDefinitionSchema`'s
+ * `strictObject({ aliases: { content: 'bodyHtml', … } })` table feeds
+ * `strictUnknownKeyError`, which runs only on the `unrecognized_keys` path and
+ * only builds a *message*; it never rewrites the key. Nor does the ADR-0087
+ * conversion layer — `packages/spec/src/conversions/registry.ts` has zero
+ * `email_template` entries, so `normalizeStackInput` emits no notice and leaves
+ * `content` exactly where the author wrote it.
+ *
+ * So the key survives to this read only through a door that skips validation
+ * (`defineStack(…, { strict: false })`, a hand-built manifest, a direct
+ * registry write) — and on precisely that path the unwrap was actively harmful,
+ * in two ways:
+ *
+ *  1. **It destroyed the author's own prescription.** With the unwrap, the
+ *     string reached `EmailTemplateDefinitionSchema.parse()` and the template
+ *     was refused with `Invalid input: expected object, received string`, and
+ *     the boot warning's `name` field came back `undefined` — the operator
+ *     could not even tell which template failed. Without it the document
+ *     reaches the parse intact and the schema answers what it was built to
+ *     answer: *Unrecognized key(s) on this email template: `content`. Did you
+ *     mean `content` → `bodyHtml`?*
+ *  2. **`content: ''` vanished.** Falsy but non-nullish, so it passed `??` and
+ *     then died at the `filter(Boolean)` below — the template was dropped with
+ *     no warning, no count, nothing (the ADR-0078 silent-loss shape).
  */
 function readDeclared(engine: any, metadataService: any, type: string): any[] {
   try {
     const reg = engine?._registry;
     if (reg?.listItems) {
-      const items = (reg.listItems(type) ?? []).map((i: any) => i?.content ?? i).filter(Boolean);
+      const items = (reg.listItems(type) ?? []).filter(Boolean);
       if (items.length > 0) return items;
     }
   } catch {
@@ -120,7 +163,7 @@ function readDeclared(engine: any, metadataService: any, type: string): any[] {
   try {
     const listed = metadataService?.list?.(type);
     const arr = typeof (listed as any)?.then === 'function' ? [] : (listed ?? []);
-    return Array.isArray(arr) ? arr.map((i: any) => i?.content ?? i).filter(Boolean) : [];
+    return Array.isArray(arr) ? arr.filter(Boolean) : [];
   } catch {
     return [];
   }

packages/plugins/plugin-email/src/email-plugin.ts

@@ -1168,7 +1168,13 @@ export class EmailServicePlugin implements Plugin {
         ? await evt.metadataService?.get?.('email_template', String(evt.name))
         : undefined);
       if (!raw) return;
-      await upsertDeclaredEmailTemplate(engine, (raw as any)?.content ?? raw, undefined, ctx.logger as any);
+      // [#8378] `evt.body` / `metadataService.get` hand back the authoring
+      // document itself — there is no `{ name, content }` envelope. Unwrapping
+      // could only have replaced the template with one of its values, and on
+      // this door `content` cannot even arrive: `saveMetaItem` validates every
+      // `email_template` write against `EmailTemplateDefinitionSchema` and
+      // refuses the key with 422 `INVALID_METADATA` before persisting.
+      await upsertDeclaredEmailTemplate(engine, raw, undefined, ctx.logger as any);
       ctx.logger.info(`EmailServicePlugin: email template '${evt.name}' materialized from a runtime write`);
     } catch (err: any) {
       ctx.logger.warn(
@@ -1228,7 +1234,9 @@ export class EmailServicePlugin implements Plugin {
     }
     if (!answered) return FAILED_READ;
     if (!item) return undefined;
-    return stripReadDecorations((item as any)?.content ?? item);
+    // [#8378] The effective read answers the template document itself; no
+    // `{ name, content }` envelope exists to unwrap on this path either.
+    return stripReadDecorations(item);
   }
 
   async dispose(): Promise<void> {

packages/plugins/plugin-security/src/bootstrap-declared-capabilities.test.ts

@@ -11,9 +11,12 @@ function makeQl(declared: any[] = []) {
     rows,
     // readDeclared() reads engine.registry.listItems(type); stub it so
     // capabilities are surfaced without a metadata service.
+    //
+    // [#8378] Items are surfaced as the real engine surfaces them — the
+    // document itself, not a `{ content: <item> }` box that nothing produces.
     registry: {
       listItems(type: string) {
-        return type === 'capability' ? declared.map((c) => ({ content: c })) : [];
+        return type === 'capability' ? [...declared] : [];
       },
     },
     async find(object: string, q: any) {
@@ -59,7 +62,7 @@ describe('bootstrapDeclaredCapabilities (ADR-0066 D1 package declaration)', () =
     await bootstrapDeclaredCapabilities(ql, null);
     // Ship a new label on the next boot.
     (ql as any).registry.listItems = (t: string) =>
-      t === 'capability' ? [{ content: { name: 'billing.refund', label: 'Issue Refund', _packageId: 'com.acme.billing' } }] : [];
+      t === 'capability' ? [{ name: 'billing.refund', label: 'Issue Refund', _packageId: 'com.acme.billing' }] : [];
     const out2 = await bootstrapDeclaredCapabilities(ql, null);
     expect(out2.seeded).toBe(0);
     expect(out2.updated).toBe(1);

packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts

@@ -57,12 +57,27 @@ interface SeedOptions {
  * items provenance-stamped with `_packageId`) is the reliable source in every
  * boot path; the metadata-service facade only surfaces these once the
  * compiled-artifact loader runs (serve.ts).
+ *
+ * [#8378] The registered item IS the authoring document — there is no
+ * `{ name, content }` envelope to unwrap, so the `i?.content ?? i` this read
+ * used to carry is gone. Same measurement #7519 made for `MetadataFacade`,
+ * re-taken at this seam: `registerMetadataCollections` (objectql `engine.ts`)
+ * registers each stack-collection element as-is, and `loadMetaFromDb` registers
+ * `convertStoredItem(JSON.parse(record.metadata))` — the parsed body, never the
+ * `sys_metadata` row. Nothing in the tree produces the envelope.
+ *
+ * Removal is a FIX, not a tidy-up. The types read through here (`permission`,
+ * `capability`, `object`) all reject `content` as an unrecognized key, so
+ * whenever the key did appear the unwrap replaced a whole authoring document
+ * with one of its values — and `''`, falsy but non-nullish, passed `??` and
+ * then died at the `filter(Boolean)` below, dropping the item with no
+ * diagnostic at all.
  */
 export function readDeclared(engine: any, type: string): any[] {
   try {
     const reg = engine?.registry;
     if (reg?.listItems) {
-      return (reg.listItems(type) ?? []).map((i: any) => i?.content ?? i).filter(Boolean);
+      return (reg.listItems(type) ?? []).filter(Boolean);
     }
   } catch { /* fall through */ }
   return [];

packages/plugins/plugin-security/src/bootstrap-declared-positions.test.ts

@@ -20,7 +20,9 @@ function makeQl(declared: any[] = []) {
   const rows: any[] = [];
   return {
     rows,
-    registry: { listItems: (type: string) => (type === 'position' ? declared.map((c) => ({ content: c })) : []) },
+    // [#8378] Items are surfaced as the real engine surfaces them — the
+    // document itself, not a `{ content: <item> }` box that nothing produces.
+    registry: { listItems: (type: string) => (type === 'position' ? [...declared] : []) },
     async find(object: string, q: any) {
       if (object !== 'sys_position') return [];
       const where = q?.where ?? {};