diff --git a/.changeset/28904-packageless-overlays-added-member.md b/.changeset/28904-packageless-overlays-added-member.md new file mode 100644 index 00000000000..5b572802ee9 --- /dev/null +++ b/.changeset/28904-packageless-overlays-added-member.md @@ -0,0 +1,14 @@ +--- +'@objectstack/metadata-protocol': patch +'@objectstack/cli': patch +--- + +fix(metadata-protocol): `os migrate packageless-overlays` moves a package-less email template row of a locale the base package does not ship into the base's `.custom` pair as the pair's own member + +Clause-②: no + +Metadata-protocol: none + +A Setup translation stored bound to no package before ADR-0146 D11, of a template the installed package ships in other locales only (a `zh-CN` row of a template shipped `en-US` only), was saved into the pair declaring `overrides`, which the override write gate refuses for a locale the base does not ship. The move failed and the row was kept. The ceremony now saves such a row into the pair with no `overrides` declaration, as the pair's own member at that locale (the base's address serves it as the pair's addition), reads it back and only then deletes the unbound row. Its subject, bodies and envelope columns move unchanged, and it keeps sending in that locale. A row of a locale the base ships moves as the declared override, as before; a declared override of a locale the base does not ship is still refused. + +The preview names such a row's locale (`addedMember` in `--json`, "its own member" in the listing), and `--apply` reports it `moved` with that `addedMember` in place of a `basedOn`, since the pair's own member records none. diff --git a/packages/cli/src/commands/migrate/packageless-overlays.ts b/packages/cli/src/commands/migrate/packageless-overlays.ts index 8e227be0d8e..825d7c1f1b4 100644 --- a/packages/cli/src/commands/migrate/packageless-overlays.ts +++ b/packages/cli/src/commands/migrate/packageless-overlays.ts @@ -93,8 +93,11 @@ function jsonRow(row: CensusRow, outcome?: Outcome): Record { ...(row.base !== undefined ? { base: row.base } : {}), ...(row.pair !== undefined ? { pair: row.pair } : {}), ...(row.shadowDeclares !== undefined ? { shadowDeclares: row.shadowDeclares } : {}), + ...(row.addedMember !== undefined ? { addedMember: row.addedMember } : {}), outcome: outcome?.outcome ?? (row.disposition === 'movable' ? 'listed' : 'left'), - ...(outcome && outcome.outcome !== 'failed' ? { packageId: outcome.packageId, basedOn: outcome.basedOn } : {}), + ...(outcome && outcome.outcome !== 'failed' + ? { packageId: outcome.packageId, ...(outcome.basedOn !== undefined ? { basedOn: outcome.basedOn } : {}) } + : {}), ...(outcome?.outcome === 'failed' ? { error: outcome.error } : {}), }; } @@ -376,7 +379,12 @@ export default class MigratePackagelessOverlays extends Command { if (!apply) { if (movable.length > 0) { console.log(chalk.bold(`${movable.length} row(s) to move:`)); - for (const row of movable) console.log(` • ${itemOf(row)} → ${row.pair} (overrides ${row.base})`); + for (const row of movable) { + // [#28904] A member the base does not ship moves in as the pair's own, declaring nothing. + console.log(` • ${itemOf(row)} → ${row.pair} ${row.addedMember !== undefined + ? `(its own member; ${row.base} ships none at ${row.addedMember})` + : `(overrides ${row.base})`}`); + } } if (result.incompleteDeclarations.length > 0) { console.log(chalk.bold(`${result.incompleteDeclarations.length} declared override(s) missing basedOn:`)); @@ -395,7 +403,8 @@ export default class MigratePackagelessOverlays extends Command { console.log(` ${chalk.red('✗')} ${itemOf(row)} — ${outcome.error}`); } else { const verb = outcome.outcome === 'moved' ? 'moved into' : 'basedOn recorded in'; - console.log(` ${chalk.green('✓')} ${itemOf(row)} ${verb} ${outcome.packageId} ${chalk.dim(`(basedOn ${outcome.basedOn})`)}`); + const detail = outcome.addedMember !== undefined ? `as its own member ${outcome.addedMember}` : `basedOn ${outcome.basedOn}`; + console.log(` ${chalk.green('✓')} ${itemOf(row)} ${verb} ${outcome.packageId} ${chalk.dim(`(${detail})`)}`); } } console.log(''); diff --git a/packages/metadata-protocol/src/protocol-packageless-overlay-migration.ts b/packages/metadata-protocol/src/protocol-packageless-overlay-migration.ts index 517eb068aa7..e5b227dec70 100644 --- a/packages/metadata-protocol/src/protocol-packageless-overlay-migration.ts +++ b/packages/metadata-protocol/src/protocol-packageless-overlay-migration.ts @@ -67,6 +67,23 @@ * `failed` outcome; the next row is still tried. A second run finds nothing to * move (idempotent). * + * ## …a bundle member the base does not ship (#28904) + * + * For a type whose identity is a pair (`ITEM_KEY_DISCRIMINATORS`: an + * `email_template` is one item per `(name, locale)`), the base may ship the + * name and not the row's member — a `zh-CN` translation of a template shipped + * `en-US` only. Such a row overrides nothing the base ships: the pair holds it + * as its OWN member, declaring nothing (ADR-0146 D2; a declared override of it + * is refused, `declared_package_ships_no_such_item`, #28777), and the base's + * address serves it as the pair's addition (#28802). The census names that + * member on the `movable` row (`addedMember`), from the shipped artifacts at + * that member as the write gate counts them ({@link addedMemberOf}), and + * the move differs in steps 2 and 3 only: the body is saved into the pair + * WITHOUT `overrides` (one it carries naming the base is dropped), so no + * `basedOn` is recorded and none is needed; the pair's row read back must be + * that member's own row, declaring nothing. Its outcome is `moved`, naming + * the `addedMember` in place of a `basedOn`. + * * ## …and the stored declarations missing `basedOn` * * A row of a pair whose body declares `overrides: { package: }` with no @@ -91,8 +108,8 @@ import { storedRowDocument } from './item-lock.js'; import { OVERLAY_MEMBER_COLUMN, overlayMemberField, overlayMemberOfBody, overlayMemberValue } from './migrations/overlay-index.js'; import type { ObjectStackProtocolImplementation } from './protocol.js'; import { customizationPairEnsureRefusal, type PairRegistry } from './protocol-customization-package.js'; -import type { OverrideWriteSubject, OverrideWriteVerdict } from './protocol-packaged-base-refusal.js'; -import { readOnlyShippingPackages } from './protocol-environment-rows.js'; +import { overrideWriteMemberOf, type OverrideWriteSubject, type OverrideWriteVerdict } from './protocol-packaged-base-refusal.js'; +import { readOnlyShippingPackages, shippingPackageIds } from './protocol-environment-rows.js'; import { isServedOverrideRowOf } from './served-override.js'; import { SysMetadataRepository } from './sys-metadata-repository.js'; @@ -138,6 +155,13 @@ export interface PackagelessOverlayRow { readonly pair?: string; /** `shadowed` only: whether the pair's row declares the override of the base. */ readonly shadowDeclares?: boolean; + /** + * [#28904] `movable` only, for a type whose identity is a pair: the bundle + * member (the row's discriminator value, e.g. its locale) the base does + * not ship. The row moves in as the pair's own member at it, declaring no + * override (module header, "a bundle member the base does not ship"). + */ + readonly addedMember?: string; } /** One pair row whose declaration lacks `basedOn` (module header, last section). */ @@ -164,7 +188,9 @@ export interface PackagelessOverlayCensus { /** What {@link migratePackagelessOverlays} did with one movable row or one incomplete declaration. */ export type PackagelessOverlayOutcome = - | { readonly id: string; readonly outcome: 'moved' | 'declaration_completed'; readonly packageId: string; readonly basedOn: string } + | { readonly id: string; readonly outcome: 'moved' | 'declaration_completed'; readonly packageId: string; readonly basedOn: string; readonly addedMember?: undefined } + /** [#28904] Moved as the pair's own member the base does not ship: no declaration, so no `basedOn`. */ + | { readonly id: string; readonly outcome: 'moved'; readonly packageId: string; readonly addedMember: string; readonly basedOn?: undefined } | { readonly id: string; readonly outcome: 'failed'; readonly error: string }; /** One base's line of the report. */ @@ -260,6 +286,28 @@ function baseVersionOf(host: Pick, ba return typeof version === 'string' && version !== '' ? version : undefined; } +/** + * [#28904, ADR-0146 D2] The bundle member `body` is, when `base` ships the + * name `name` but not that member — the member the row adds to the pair; `undefined` for a type whose identity is + * its name alone, and for a member `base` ships. Counted as the override + * write gate counts a shipper (`overrideWriteVerdict`): the artifacts at that + * member, less any another member answers (a registry that ignores `member` + * can answer one), bound to `base` — ⛔ never a second reading. + */ +function addedMemberOf( + host: Pick, + type: string, + name: string, + base: string, + body: unknown, +): string | undefined { + const member = overrideWriteMemberOf(type, body); + if (member === undefined) return undefined; + const atMember = host.shippedArtifactsOf(type, name, undefined, member) + .filter((artifact) => overrideWriteMemberOf(type, artifact) === member); + return shippingPackageIds(atMember).has(base) ? undefined : member; +} + /** * [#28576] How the census classifies ONE package-less row of a name some * read-only code package ships (module header, "The population"), from the @@ -345,8 +393,12 @@ export async function listPackagelessOverlays( ...(member !== undefined ? { member } : {}), shippers, }; - const classified = classifyPackagelessRow(shippers, bodyOf(row), (pair) => shadowOf(pair, type, name, member, state)); - listed.push({ ...common, ...classified }); + const body = bodyOf(row); + const classified = classifyPackagelessRow(shippers, body, (pair) => shadowOf(pair, type, name, member, state)); + const addedMember = classified.disposition === 'movable' && classified.base !== undefined + ? addedMemberOf(this, type, name, classified.base, body) + : undefined; + listed.push({ ...common, ...classified, ...(addedMember !== undefined ? { addedMember } : {}) }); } // Every type a declaration applies to, not only the five the move acts on. @@ -393,6 +445,12 @@ async function storedPairRow( return (rows ?? []).find((row) => memberOf(row) === item.member); } +/** [#28904] Whether a pair's stored row is its own item: a document that declares no override. */ +function isOwnMemberRow(row: unknown): boolean { + const body = row === undefined ? undefined : bodyOf(row); + return body !== undefined && !('overrides' in body); +} + /** * Move every `movable` row of the census into its base's pair, and complete * every declaration missing `basedOn` (module header, "The move"). One outcome @@ -451,25 +509,33 @@ export async function migratePackagelessOverlays( }; const ensuredPair = await ensure(row.base, row.pair); if (!ensuredPair.ok) { fail(ensuredPair.error); continue; } - const basedOn = baseVersionOf(this, row.base); - if (basedOn === undefined) { fail(`The installed package '${row.base}' declares no version, so no basedOn can be recorded.`); continue; } + // [#28904] A member the base does not ship is the pair's own: no + // declaration, so no `basedOn` to record. + const { addedMember } = row; + const basedOn = addedMember === undefined ? baseVersionOf(this, row.base) : undefined; + if (addedMember === undefined && basedOn === undefined) { fail(`The installed package '${row.base}' declares no version, so no basedOn can be recorded.`); continue; } const stored = rowsById.get(row.id); const body = stored === undefined ? undefined : bodyOf(stored); if (body === undefined) { fail('The stored body could not be read as a document.'); continue; } try { - // Step 2: the declared body, saved into the pair through the save door. + // Step 2: the declared body — or, for an added member, the body + // with no `overrides` — saved into the pair through the save door. + const { overrides: _declared, ...undeclared } = body; await this.saveMetaItem({ type: row.type, name: row.name, - item: { ...body, overrides: { package: row.base, basedOn } }, + item: addedMember === undefined ? { ...body, overrides: { package: row.base, basedOn } } : undeclared, packageId: row.pair, mode: row.state === 'draft' ? 'draft' : 'publish', actor: request.actor, } as never); - // Step 3: read back — the pair's row must be the served override. + // Step 3: read back — the pair's row must be the served override, + // or, for an added member, that member's own row declaring nothing. const written = await storedPairRow(this, row, row.pair); - if (!isServedOverrideRowOf(written, row.base)) { - fail(`The row saved into '${row.pair}' does not read back as the declared override of '${row.base}'; the unbound row is kept.`); + if (addedMember === undefined ? !isServedOverrideRowOf(written, row.base) : !isOwnMemberRow(written)) { + fail(addedMember === undefined + ? `The row saved into '${row.pair}' does not read back as the declared override of '${row.base}'; the unbound row is kept.` + : `The row saved into '${row.pair}' does not read back as its own member '${addedMember}', declaring no override; the unbound row is kept.`); continue; } // Step 4: only now, the unbound row, by its own address. @@ -479,7 +545,9 @@ export async function migratePackagelessOverlays( parentVersion: (typeof stored.checksum === 'string' ? stored.checksum : null) as string, actor: request.actor, source: request.actor, - message: `Moved into '${row.pair}' as a declared override of '${row.base}' (ADR-0146 D11)`, + message: addedMember === undefined + ? `Moved into '${row.pair}' as a declared override of '${row.base}' (ADR-0146 D11)` + : `Moved into '${row.pair}' as its own member '${addedMember}', which '${row.base}' does not ship (ADR-0146 D2, D11)`, intent: 'override-artifact', state: row.state, packageId: null, @@ -487,7 +555,9 @@ export async function migratePackagelessOverlays( } as never, ); line.moved += 1; - outcomes.push({ id: row.id, outcome: 'moved', packageId: row.pair, basedOn }); + outcomes.push(addedMember === undefined + ? { id: row.id, outcome: 'moved', packageId: row.pair, basedOn: basedOn! } + : { id: row.id, outcome: 'moved', packageId: row.pair, addedMember }); } catch (error) { fail(messageOf(error)); } @@ -584,8 +654,11 @@ export interface PackagelessWriteSubject { * takes: the ensure door would ensure the pair * ({@link customizationPairEnsureRefusal}), the base records a version * (`basedOn`), and the override write gate admits the declared body into - * the pair (an `email_template` member at a locale the base does not ship - * is refused there, #28777, and stays open: #28904). + * the pair. An `email_template` member at a locale the base does not ship + * is refused there (#28777) and stays open: the ceremony moves such a row + * as the pair's own member, declaring nothing (#28904), and the close + * reaches only the declared-override moves, so its package-less write is + * the one it had before the close. * * Registry reads first; the one store read (the item's rows) only for a name * the registry has not already left open. diff --git a/packages/objectql/src/environment-overlay-email-locale-restart.test.ts b/packages/objectql/src/environment-overlay-email-locale-restart.test.ts index 3af91d25523..48232ce4fbc 100644 --- a/packages/objectql/src/environment-overlay-email-locale-restart.test.ts +++ b/packages/objectql/src/environment-overlay-email-locale-restart.test.ts @@ -32,7 +32,8 @@ * row; a package-less row is laid down as a release before the close stored * it (`seedPackagelessRow`), served until the ceremony moves it; a shipped * member's package-less write is refused, and an unshipped member's, which - * the ceremony cannot move (#28904), stays open. + * the ceremony moves as the pair's own member declaring nothing (#28904, the + * last section), stays open. * * ## The harness * @@ -865,8 +866,10 @@ describe('[#23074] the artifact envelope is stamped only on the member the packa * the by-name read then serve, or `'closed'`: the ceremony moves a shipped * member's unmigrated row, so its package-less reset is refused naming the * ceremony and the row keeps serving its member. A member the base does - * not ship cannot be moved (the pair's save refuses it, #28777; #28904), - * so its package-less reset stays open, as before the close. + * not ship moves as the pair's own member, declaring nothing (a declared + * override of it is refused, #28777; #28904), and the close reaches only + * the declared-override moves, so its package-less reset stays open, as + * before the close. */ async function resetZh(name: string, verbs: { shadow: boolean; overlayEntry: boolean }, after: string | undefined | 'closed') { const shadowBoot = (await bootWithOverlays(new Map(), name)).engine; @@ -945,3 +948,122 @@ describe('[#23074] the artifact envelope is stamped only on the member the packa expect(read.packageId).toBe(PKG); }); }); + +/** + * [#28904, ADR-0146 D2 / D11] `os migrate packageless-overlays` and a + * package-less row of a member the package does not ship. The package ships + * `acme.digest` in `en-US` alone, so a `zh-CN` row of it overrides nothing + * the package ships: the pair holds it only as its OWN member, declaring no + * override (a declared override of it stays refused, #28777). The ceremony + * moves it that way and reports it `moved`, naming the member it adds; a + * shipped member's row (`acme.welcome`'s `zh-CN`) still moves as the declared + * override. Either way the send path keeps the row's wording and envelope + * columns, the unbound row is gone, and a second run moves nothing. + */ +describe('[#28904] the ceremony moves a row of a member the package does not ship as the pair\'s own member', () => { + const ACTOR = 'os migrate packageless-overlays'; + const ENVELOPE = { + bodyText: 'ADMIN digest zh-CN text', + replyTo: 'support@acme.test', + fromOverride: { name: 'Acme 支持', address: 'support@acme.test' }, + }; + const rowsOf = (stores: Map>>, name: string) => + Array.from(stores.get('sys_metadata')?.values() ?? []) + .filter((r) => r.type === T && r.name === name) + .map((r) => ({ + packageId: (r.package_id as string | null | undefined) ?? null, + member: (r.bundle_member as string | null | undefined) ?? null, + state: r.state, + body: JSON.parse(String(r.metadata)) as Record, + })); + /** The member the send path resolves (`plugin-email`'s loader reads the execution list by `(name, locale)`). */ + const sentMember = async (protocol: ObjectStackProtocolImplementation, name: string, locale: string) => + (((await protocol.getMetaItemsForExecution({ type: T })) as any)?.items as any[] ?? []) + .find((it) => it?.name === name && it?.locale === locale); + const census = (protocol: ObjectStackProtocolImplementation): Promise => (protocol as any).listPackagelessOverlays(); + const migrate = (protocol: ObjectStackProtocolImplementation): Promise => + (protocol as any).migratePackagelessOverlays({ actor: ACTOR }); + + /** One package-less row of `name` in `zh-CN`, stored by a release before the close, then a restart. */ + async function withUnboundZh(name: string, extra: Record = {}) { + const stores = new Map>>(); + const first = await boot(stores); + await seedPackagelessRow(first.raw, name, { ...template('zh-CN', `ADMIN ${name} zh-CN`), name, ...ENVELOPE, ...extra }); + return { stores, ...(await boot(stores)) }; + } + + it('an unshipped locale: moved into the pair as its own member with no `overrides`, still sent with its wording and envelope, and a re-run moves 0', async () => { + const { stores, raw } = await withUnboundZh(DIGEST); + const sent = { subject: `ADMIN ${DIGEST} zh-CN`, bodyHtml: `

ADMIN ${DIGEST} zh-CN

`, ...ENVELOPE }; + expect(await sentMember(raw, DIGEST, 'zh-CN')).toMatchObject(sent); + + const listed = await census(raw); + expect(listed.rows.map((r: any) => [r.name, r.member, r.disposition, r.addedMember])).toEqual([[DIGEST, 'zh-CN', 'movable', 'zh-CN']]); + + const result = await migrate(raw); + expect(result.outcomes).toEqual([{ id: listed.rows[0].id, outcome: 'moved', packageId: PAIR, addedMember: 'zh-CN' }]); + expect(result.bases).toEqual([{ base: PKG, pair: PAIR, pairCreated: false, moved: 1, declarationsCompleted: 0, failed: 0, shadowed: 0 }]); + const stored = rowsOf(stores, DIGEST); + expect(stored.map((r) => [r.packageId, r.member, r.state])).toEqual([[PAIR, 'zh-CN', 'active']]); + expect(stored[0]!.body).not.toHaveProperty('overrides'); + expect(stored[0]!.body).toMatchObject(sent); + + // Still sent, at once and after a restart; the package's own `en-US` member is untouched. + for (const served of [raw, (await boot(stores)).raw]) { + expect(await sentMember(served, DIGEST, 'zh-CN')).toMatchObject(sent); + expect(((await served.getMetaItem({ type: T, name: DIGEST, member: 'zh-CN' })) as any)?.item).toMatchObject(sent); + expect((await sentMember(served, DIGEST, 'en-US'))?.subject).toBe('PKG digest en'); + } + + const again = await migrate(raw); + expect(again.rows).toEqual([]); + expect(again.outcomes).toEqual([]); + expect(rowsOf(stores, DIGEST).map((r) => [r.packageId, r.member, r.state])).toEqual([[PAIR, 'zh-CN', 'active']]); + }); + + it('an unshipped locale whose stored body declares the base: the declaration is dropped, as the pair\'s own member carries none', async () => { + const { stores, raw } = await withUnboundZh(DIGEST, { overrides: { package: PKG, basedOn: '0.9.0' } }); + const result = await migrate(raw); + expect(result.outcomes.map((o: any) => [o.outcome, o.addedMember])).toEqual([['moved', 'zh-CN']]); + const stored = rowsOf(stores, DIGEST); + expect(stored.map((r) => [r.packageId, r.member])).toEqual([[PAIR, 'zh-CN']]); + expect(stored[0]!.body).not.toHaveProperty('overrides'); + expect((await sentMember(raw, DIGEST, 'zh-CN'))?.subject).toBe(`ADMIN ${DIGEST} zh-CN`); + }); + + it('READ BACK FIRST: a save that leaves no row of that member in the pair keeps the unbound row, and the row is reported failed', async () => { + const { stores, raw } = await withUnboundZh(DIGEST); + // A save door that answers and writes nothing. + (raw as any).saveMetaItem = async () => ({ success: true }); + const result = await migrate(raw); + expect(result.outcomes.map((o: any) => o.outcome)).toEqual(['failed']); + expect(result.outcomes[0].error).toContain("does not read back as its own member 'zh-CN'"); + expect(rowsOf(stores, DIGEST).map((r) => [r.packageId, r.member])).toEqual([[null, 'zh-CN']]); + expect(result.bases[0]).toMatchObject({ moved: 0, failed: 1 }); + }); + + it('CONTROL a shipped locale: moved as the declared override of the package, as before', async () => { + const { stores, raw } = await withUnboundZh(NAME); + const listed = await census(raw); + expect(listed.rows.map((r: any) => [r.name, r.member, r.disposition])).toEqual([[NAME, 'zh-CN', 'movable']]); + expect(listed.rows[0]).not.toHaveProperty('addedMember'); + + const result = await migrate(raw); + expect(result.outcomes).toEqual([{ id: listed.rows[0].id, outcome: 'moved', packageId: PAIR, basedOn: '1.0.0' }]); + const stored = rowsOf(stores, NAME); + expect(stored.map((r) => [r.packageId, r.member, r.state])).toEqual([[PAIR, 'zh-CN', 'active']]); + expect(stored[0]!.body.overrides).toEqual({ package: PKG, basedOn: '1.0.0' }); + expect(await sentMember(raw, NAME, 'zh-CN')).toMatchObject({ subject: `ADMIN ${NAME} zh-CN`, ...ENVELOPE }); + }); + + it('the refusal of a declared override at an unshipped locale is unchanged: after the move, that save into the pair answers 403 NOT_OVERRIDABLE', async () => { + const { stores, raw } = await withUnboundZh(DIGEST); + await migrate(raw); + const refused: any = await raw.saveMetaItem({ + type: T, name: DIGEST, packageId: PAIR, + item: { ...template('zh-CN', 'DECLARED zh-CN'), name: DIGEST, overrides: DECLARATION }, + } as any).then(() => undefined, (e: unknown) => e); + expect(refused).toMatchObject({ code: 'NOT_OVERRIDABLE', status: 403 }); + expect(rowsOf(stores, DIGEST).map((r) => r.body.subject)).toEqual([`ADMIN ${DIGEST} zh-CN`]); + }); +});