Skip to content

Commit eeccde6

Browse files
committed
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
1 parent f5aec38 commit eeccde6

2 files changed

Lines changed: 153 additions & 25 deletions

File tree

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

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

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)