Skip to content

Commit dfb7a0d

Browse files
os-warrenclaude
os-warren
and
claude
authored
fix(plugin-email): use the shared read-decoration strip, not a blanket underscore sweep (#16239)
* fix(plugin-email): use the shared read-decoration strip, not a blanket `_` sweep `readEffectiveTemplate` stripped read decorations with a module-local copy of `stripReadDecorations` that dropped every key starting with `_`. The shared list it drifted from (`spec/kernel/metadata-read-decorations.ts`) carries exactly `['_diagnostics', '_draft']` and names the ADR-0010 protection envelope as "Deliberately NOT" a member — envelope state the write path legitimately carries, allowlisted by the closed schemas. The copy justified the sweep on the claim that `EmailTemplateDefinitionSchema` "declares no underscore key". `email-template.zod.ts` spreads `MetadataProtectionFields` into its `strictObject`, so every envelope key is declared and parses clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y * test(plugin-email): correct a `_provenance` fixture the blanket sweep was hiding `_provenance: { source: 'code' }` is an object where `MetadataProvenanceSchema` is `z.enum(['package','org','env-forced'])`. The fixture was never spec-legal; the module-local blanket `_` sweep deleted it before the parse could say so. With the shared strip it reaches the schema and is rejected — the silent swallow, demonstrated on the repo's own fixture. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y * chore(changeset): plugin-email read-decoration strip Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y * test(plugin-email): pay the spec load at module top, per check:test-source-alias `check:test-source-alias` flagged the dynamic `import('@objectstack/spec/system')` inside a test body: this package resolves that specifier through `dist/`, so the first call transforms the dependency's whole module graph inside a clocked window. Hoisted to a module-scope import, where vitest clocks nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 1b5bc6b commit dfb7a0d

3 files changed

Lines changed: 172 additions & 26 deletions

File tree

‎.changeset/email-template-shared-read-decoration-strip.md‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
"@objectstack/plugin-email": patch
3+
---
4+
5+
`plugin-email` strips read decorations with the shared list, not a blanket underscore sweep.
6+
7+
`readEffectiveTemplate` — the layered read a `DELETE /meta/email_template/:name` runs to restore the packaged baseline an overlay was hiding — removed decorations with a module-local copy of `stripReadDecorations` that dropped **every** key beginning with `_`. The shared list it drifted from, `METADATA_READ_DECORATIONS` in `@objectstack/spec/kernel`, is exactly `['_diagnostics', '_draft']`, and its module header names the ADR-0010 protection envelope (`_lock`, `_lockReason`, `_lockSource`, `_provenance`, `_packageId`, `_packageVersion`, `_lockDocsUrl`) as deliberately **not** a member: it is envelope state the write path legitimately carries, and the closed metadata schemas allowlist it so a served document keeps its provenance on re-parse.
8+
9+
The private copy justified its sweep on the claim that `EmailTemplateDefinitionSchema` "declares no underscore key". That is false — `email-template.zod.ts` spreads `MetadataProtectionFields` into its `strictObject`, so every envelope key is declared and parses clean. The copy was removing keys the schema was deliberately widened to accept, and the list lives in `spec` precisely so a producer and its consumers cannot drift like this.
10+
11+
The path now calls the shared helper, matching the other read-back-envelope consumers (the dataset query in `rest-server.ts`, the cold-boot flow bind in `service-automation`, `saveMetaItem`'s verbatim persist, and the route-level seed apply). Two behavioural consequences:
12+
13+
- An underscore key that is neither a decoration nor declared is no longer swallowed before validation. The closed schemas exist to reject exactly that (protocol 17), and the rejection is now reported on the write's own response through the mutation projector, instead of the reset quietly succeeding against a body the schema would have refused.
14+
- The ADR-0010 envelope survives the strip. It still does not reach `sys_email_template`: `upsertDeclaredEmailTemplate` projects the parsed template through `mapTemplateToRow`, a closed column list, and the object declares no underscore column — so no stored row changes shape. There is deliberately no second, envelope-stripping pass beside the shared one; spelling one would re-create the drift this fixes, one layer up.

‎packages/plugins/plugin-email/src/email-plugin.template-runtime-write.test.ts‎

Lines changed: 119 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
import { describe, it, expect, vi } from 'vitest';
2929
import { assertEngineUpdateDispatch } from '@objectstack/objectql';
30+
import { EmailTemplateDefinitionSchema } from '@objectstack/spec/system';
3031
import { EmailServicePlugin } from './email-plugin.js';
3132

3233
const TABLE = 'sys_email_template';
@@ -263,16 +264,26 @@ describe('#7733 runtime email_template write materializes without a restart', ()
263264
// `getMetaItem` returns a DECORATED item (`_diagnostics` from
264265
// `decorateMetadataItem`, `_packageId` / `_provenance` from the registry
265266
// and the overlay row). `EmailTemplateDefinitionSchema` is a strictObject
266-
// that declares no underscore key, so an unstripped body would reject the
267-
// very baseline the reset exists to restore.
267+
// that declares neither `_diagnostics` nor `_draft`, so an unstripped body
268+
// would reject the very baseline the reset exists to restore.
269+
//
270+
// [#16152] It DOES declare the ADR-0010 envelope (`_packageId` /
271+
// `_provenance` and the `_lock*` family, via `MetadataProtectionFields`) —
272+
// those parse clean and are not stripped. See the `#16152` describe block
273+
// below for the pins that hold that apart.
268274
const protocol = withEffectiveRead(fakeProtocol(), async () => ({
269275
type: 'email_template',
270276
name: 'auth.password_reset',
271277
item: {
272278
...template({ subject: 'The packaged subject' }),
273279
_diagnostics: { valid: true },
274280
_packageId: 'com.objectstack.auth',
275-
_provenance: { source: 'code' },
281+
// [#16152] Was `{ source: 'code' }` — an object, where
282+
// `MetadataProvenanceSchema` is `z.enum(['package','org','env-forced'])`.
283+
// It was never spec-legal; the blanket `_` sweep deleted it before the
284+
// parse could say so, which is the silent swallow this card removes.
285+
// Corrected to the spelling the rest of the repo's fixtures use.
286+
_provenance: 'package',
276287
},
277288
}));
278289
const { engine } = await boot({ protocol });
@@ -378,3 +389,108 @@ describe('#7733 runtime email_template write materializes without a restart', ()
378389
expect(engine.rows).toHaveLength(0);
379390
});
380391
});
392+
393+
// ── #16152 ─────────────────────────────────────────────────────────────────
394+
//
395+
// `readEffectiveTemplate` used to strip read decorations with a MODULE-LOCAL
396+
// copy of `stripReadDecorations` that dropped every key starting with `_`.
397+
// The shared list it drifted from (`spec/kernel/metadata-read-decorations.ts`)
398+
// carries exactly `['_diagnostics', '_draft']` and names the ADR-0010
399+
// protection envelope as "Deliberately NOT" a member — envelope state the
400+
// write path legitimately carries, allowlisted by the closed schemas so a
401+
// served document keeps its provenance on re-parse.
402+
//
403+
// The copy's docblock justified the blanket sweep on the claim that
404+
// `EmailTemplateDefinitionSchema` "declares no underscore key". These pin why
405+
// that claim is false and what the narrow strip must do instead.
406+
407+
describe('#16152 read-decoration strip uses the shared list, not a blanket `_` sweep', () => {
408+
/** The full ADR-0010 envelope, as `MetadataProtectionFields` declares it. */
409+
const ENVELOPE = {
410+
_lock: 'no-delete',
411+
_lockReason: 'Shipped by the auth package.',
412+
_lockSource: 'artifact',
413+
_provenance: 'package',
414+
_packageId: 'com.objectstack.auth',
415+
_packageVersion: '1.4.2',
416+
_lockDocsUrl: 'https://example.invalid/locks',
417+
} as const;
418+
419+
it('the schema declares the ADR-0010 envelope and rejects the read decorations', () => {
420+
// The premise the deleted docblock got backwards, asserted directly
421+
// against the schema rather than inferred from it. `email-template.zod.ts`
422+
// spreads `MetadataProtectionFields` into its `strictObject`, so every
423+
// envelope key is authorable surface here; `_diagnostics` / `_draft` are
424+
// not declared anywhere in that shape, which is why they must be stripped.
425+
const withEnvelope = EmailTemplateDefinitionSchema.safeParse({ ...template(), ...ENVELOPE });
426+
expect(withEnvelope.success).toBe(true);
427+
// Guarding a KEY's reachability: no `unrecognized_keys` on any envelope key.
428+
expect(
429+
(withEnvelope as any).error?.issues?.filter((i: any) => i.code === 'unrecognized_keys') ?? [],
430+
).toEqual([]);
431+
432+
for (const decoration of ['_diagnostics', '_draft']) {
433+
const served = EmailTemplateDefinitionSchema.safeParse({
434+
...template(),
435+
[decoration]: decoration === '_draft' ? true : { valid: true },
436+
});
437+
expect(served.success).toBe(false);
438+
const keys = (served as any).error.issues
439+
.filter((i: any) => i.code === 'unrecognized_keys')
440+
.flatMap((i: any) => i.keys);
441+
expect(keys).toContain(decoration);
442+
}
443+
});
444+
445+
it('re-materializes a baseline served with the FULL protection envelope on it', async () => {
446+
// The envelope rides along on the layered read (`_packageId` /
447+
// `_provenance` from the registry, `_lock*` from the artifact layer). It
448+
// parses clean, so the reset restores the packaged baseline with no
449+
// projector failure — and `mapTemplateToRow`'s closed column list is what
450+
// keeps it out of the row, not a strip.
451+
const protocol = withEffectiveRead(fakeProtocol(), async () => ({
452+
item: {
453+
...template({ subject: 'The packaged subject' }),
454+
...ENVELOPE,
455+
_diagnostics: { valid: true },
456+
_draft: false,
457+
},
458+
}));
459+
const { engine } = await boot({ protocol });
460+
461+
await protocol.save('auth.password_reset', template({ subject: 'An operator override' }));
462+
await protocol.remove('auth.password_reset');
463+
464+
expect(protocol.projectorFailures).toEqual([]);
465+
const rows = rowsOf(engine, 'auth.password_reset');
466+
expect(rows).toHaveLength(1);
467+
expect(rows[0].subject).toBe('The packaged subject');
468+
// `sys_email_template` declares no underscore column, and
469+
// `mapTemplateToRow` projects a closed list — so the envelope cannot reach
470+
// the row whatever the strip does. This is the measured reason there is no
471+
// second, envelope-stripping pass beside the shared one.
472+
for (const k of Object.keys(ENVELOPE)) expect(rows[0]).not.toHaveProperty(k);
473+
});
474+
475+
it('does not silently swallow an underscore key the schema never declared', async () => {
476+
// The blanket sweep dropped EVERY `_` key before the parse, so a key that
477+
// is neither a decoration nor declared — a producer's typo, a decoration
478+
// added upstream and never added to the shared list — vanished and the
479+
// reset reported success. That is precisely the silent-strip failure the
480+
// closed schemas (#4001) exist to end, and the shared list keeps loud:
481+
// the projector surfaces it on the write's own response.
482+
const protocol = withEffectiveRead(fakeProtocol(), async () => ({
483+
item: { ...template({ subject: 'The packaged subject' }), _notADeclaredKey: 'x' },
484+
}));
485+
const { engine } = await boot({ protocol });
486+
487+
await protocol.save('auth.password_reset', template({ subject: 'An operator override' }));
488+
await protocol.remove('auth.password_reset');
489+
490+
expect(protocol.projectorFailures).toHaveLength(1);
491+
expect(protocol.projectorFailures[0]).toContain('_notADeclaredKey');
492+
// The override row is left exactly as it was — a body the schema refuses
493+
// never becomes a write.
494+
expect(rowsOf(engine, 'auth.password_reset')[0].subject).toBe('An operator override');
495+
});
496+
});

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

Lines changed: 39 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import type { Plugin, PluginContext } from '@objectstack/core';
44
import type { IDataEngine } from '@objectstack/spec/contracts';
5+
import { stripReadDecorations } from '@objectstack/spec/kernel';
56
import type {
67
IEmailTransport,
78
EmailAddress,
@@ -66,23 +67,6 @@ const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const;
6667
*/
6768
const FAILED_READ = Symbol('email-template-read-failed');
6869

69-
/**
70-
* Drop the underscore-prefixed keys a SERVED metadata item carries
71-
* (`_diagnostics`, `_packageId`, `_provenance`, `_draft`, …). Every one of
72-
* them is a read-time verdict the protocol attaches, never an authored field:
73-
* `EmailTemplateDefinitionSchema` is a `strictObject` and declares no
74-
* underscore key, so leaving them on turns a perfectly good declaration into a
75-
* validation failure.
76-
*/
77-
function stripReadDecorations(item: unknown): unknown {
78-
if (!item || typeof item !== 'object' || Array.isArray(item)) return item;
79-
const out: Record<string, unknown> = {};
80-
for (const [k, v] of Object.entries(item as Record<string, unknown>)) {
81-
if (!k.startsWith('_')) out[k] = v;
82-
}
83-
return out;
84-
}
85-
8670
/**
8771
* Plugin configuration.
8872
*/
@@ -1282,13 +1266,45 @@ export class EmailServicePlugin implements Plugin {
12821266
* registry, which for a delete is the same baseline. The three outcomes are
12831267
* kept apart on purpose: only "nothing declares it" may deactivate a row.
12841268
*
1285-
* The body is stripped of read decorations before it is returned. A served
1286-
* item carries the protocol's own underscore keys (`_diagnostics` from
1287-
* `decorateMetadataItem`, `_packageId` / `_provenance` from the registry and
1288-
* the overlay row), `EmailTemplateDefinitionSchema` is a `strictObject`, and
1289-
* it declares no underscore key — so handing the decorated body to the
1290-
* upsert would reject the very baseline this read exists to restore. This is
1269+
* The body is stripped of read decorations before it is returned, using the
1270+
* SHARED `stripReadDecorations` from `@objectstack/spec/kernel` — the same
1271+
* helper the other read-back-envelope consumers call (the dataset query in
1272+
* `rest-server.ts`, the cold-boot flow bind in `service-automation`,
1273+
* `saveMetaItem`'s verbatim persist, and the route-level seed apply). This is
12911274
* the read-side twin of the strip `saveMetaItem` does on the write side.
1275+
*
1276+
* ## [#16152] Why this is the shared list and NOT a blanket `_`-sweep
1277+
*
1278+
* This call used to be a module-local copy that dropped **every** key
1279+
* starting with `_`, justified in its own docblock by the claim that
1280+
* `EmailTemplateDefinitionSchema` "declares no underscore key". That claim is
1281+
* false, and the schema disagrees in both directions:
1282+
*
1283+
* - `METADATA_READ_DECORATIONS` is `['_diagnostics', '_draft']`, and those
1284+
* two genuinely must go: the schema is a `strictObject`, neither key is
1285+
* declared, so an unstripped body rejects the very baseline this read
1286+
* exists to restore.
1287+
* - The ADR-0010 protection envelope (`_lock`, `_lockReason`, `_lockSource`,
1288+
* `_provenance`, `_packageId`, `_packageVersion`, `_lockDocsUrl`) is
1289+
* **declared** — `email-template.zod.ts` spreads `MetadataProtectionFields`
1290+
* into the shape — and `metadata-read-decorations.ts` names it
1291+
* "Deliberately NOT" a member of the strip list, because it is envelope
1292+
* state the write path legitimately carries. Sweeping it here removed keys
1293+
* the schema was deliberately widened to accept.
1294+
*
1295+
* A blanket sweep is also a silent swallow: an underscore key that is neither
1296+
* a decoration nor declared is exactly what the closed schema exists to
1297+
* reject (#4001), and dropping it before the parse converts a loud
1298+
* `unrecognized_keys` into a quiet success.
1299+
*
1300+
* ⭐ There is deliberately **no** second, envelope-stripping pass beside this
1301+
* one. The envelope cannot reach the written row in the first place:
1302+
* `upsertDeclaredEmailTemplate` projects the parsed template through
1303+
* `mapTemplateToRow`, a closed column list, and `sys_email_template` declares
1304+
* no underscore column. Spelling an envelope strip here would encode a rule
1305+
* this path does not have, as a second copy of a truth that already lives in
1306+
* that projection — which is the drift this fix removes, re-introduced one
1307+
* layer up.
12921308
*/
12931309
private async readEffectiveTemplate(
12941310
ctx: PluginContext,

0 commit comments

Comments
 (0)