diff --git a/.changeset/23297-package-axis-delete-draft.md b/.changeset/23297-package-axis-delete-draft.md new file mode 100644 index 00000000000..7063bedaa74 --- /dev/null +++ b/.changeset/23297-package-axis-delete-draft.md @@ -0,0 +1,21 @@ +--- +'@objectstack/metadata-protocol': minor +--- + +fix(metadata-protocol)!: a delete naming one package keeps the item's other rows registered, a draft save that names no package of an item two or more packages hold is refused `409 RESOURCE_CONFLICT`, and the delete's `_lock` refusal carries the named package's reason (ADR-0048) + +Clause-②: yes (narrowing) + + + +**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes. + +A `sys_metadata` row is keyed by its package (ADR-0048), so one item can hold one row per package. Since the save and the delete name their package (`?package=` on the REST door, `packageId` in process), three paths still did not take the package they were handed. + +**The registry after a delete naming one package.** FROM: `DELETE /api/v1/meta/object/my_object?package=com.acme.a` of an object two packages hold removed `com.acme.a`'s row and the receipt said the other package's row remains, and the item read served that row, but the registry entry was retired as if the deleted row were the whole item. Data CRUD then answered `404 OBJECT_NOT_FOUND` for the object until the next boot or save, and on a kernel whose registry holds the stored rows, registry lookups missed an item of any other type the same way. TO: when a row of the item is still stored after a delete, that row is registered again, as a boot registers it, and nothing is retired. Deleting the last row still retires the item. + +**A draft save that names no package.** FROM: `PUT /api/v1/meta/view/my_view?mode=draft` with no `?package=`, of an item whose active rows two packages hold, answered `200` "(env-wide, state=draft)" and stored the draft in whichever package the database returned first. With one package and a package-less row beside it, the draft went into the package or stayed package-less by the same order. TO: the draft inherits the package of the item's active rows when they are bound to one package, whatever the storage order (a package-less row beside it is not a second package). When they are bound to two or more, the save is refused `409 RESOURCE_CONFLICT`, writes nothing, and names the packages with the `?package=` remedy, exactly as the direct save and the delete answer. A draft read that names no package serves the package-unbound draft's version for such an item. + +**The delete's `_lock` refusal.** The delete's lock gate now takes the package the delete names, as the save's does. The verdict is unchanged: the lock is the strictest of every package that ships or stores the item, so a delete naming a package is refused whenever the save naming it is. What changes is the prose: when several packages declare the strictest lock, the `ITEM_LOCKED` refusal of a delete naming a package carries that package's reason, where it carried the reason of the package whose id sorts first. + +**Remedy.** For an item two or more packages hold, send the draft save with `?package=` naming the package whose draft it is (`packageId` in process). Nothing else asks anything of a caller. diff --git a/packages/metadata-protocol/src/protocol.object-registry-write-through-spelling.test.ts b/packages/metadata-protocol/src/protocol.object-registry-write-through-spelling.test.ts index b3756dbe4b3..a40a9a051ec 100644 --- a/packages/metadata-protocol/src/protocol.object-registry-write-through-spelling.test.ts +++ b/packages/metadata-protocol/src/protocol.object-registry-write-through-spelling.test.ts @@ -38,7 +38,7 @@ // The trace, measured on `origin/main` — all four routes fold at the producer // --------------------------------------------------------------------------- // `applyObjectRegistryMutation` has exactly ONE caller -// (`applyRegistryWriteThrough`), which has exactly SIX: +// (`applyRegistryWriteThrough`), which has exactly SEVEN: // // 1. `saveMetaItem` passes `singularTypeForRepo`, and the method // already ran `canonicalizeMetaRequestType`. @@ -66,6 +66,12 @@ // `'view'`. It never passes `'object'`, so it // never reaches `applyObjectRegistryMutation`: // an orphaned object is rebound in storage only. +// 7. `restoreArtifactRegistryView` (#23297) folds at the call site through +// `canonicalMetaType`, the key it also reads +// the stored rows under, and registers a row +// still stored after a delete (another +// package's, or the package-less one), so the +// registry keeps serving what the store serves. // // Both fold maps resolve the plural — `PLURAL_TO_SINGULAR.objects === 'object'` // and `canonicalMetaUrlType('objects') === 'object'` — so no route can deliver @@ -567,9 +573,9 @@ describe('[#8862] the `applyRegistryWriteThrough` caller set stays closed', () = 'utf8', ); - it('has exactly six call sites, all traced in this file’s header', () => { + it('has exactly seven call sites, all traced in this file’s header', () => { const callSites = source.match(/this\.applyRegistryWriteThrough\(/g) ?? []; - expect(callSites).toHaveLength(6); + expect(callSites).toHaveLength(7); }); it('`applyObjectRegistryMutation` is reached only through the write-through', () => { diff --git a/packages/metadata-protocol/src/protocol.packageless-save-address.test.ts b/packages/metadata-protocol/src/protocol.packageless-save-address.test.ts index 9dc06e0a77d..4e12caad209 100644 --- a/packages/metadata-protocol/src/protocol.packageless-save-address.test.ts +++ b/packages/metadata-protocol/src/protocol.packageless-save-address.test.ts @@ -595,3 +595,94 @@ describe('CONTROL: what the delete resolution leaves alone', () => { expect((await protocol.getMetaItem({ type: 'app', name })).item).toMatchObject({ active: false }); }); }); + +/** + * [#23297, ADR-0048] The DRAFT save naming no package, of an item two packages + * hold. + * + * The draft save resolves its address in the repository: a draft that names no + * package is stored in the package of the item's active row (#11087, + * `SysMetadataRepository.resolveWriteHead`). That resolution read ONE active + * row with a package-agnostic `findOne`, so with two packages' rows the draft + * went into whichever package the store returned first, and the save answered + * `200` "(env-wide, state=draft)" without naming it. Measured on a booted + * showcase (SQLite): the draft landed in the package whose id sorts first, + * whichever package was saved first. The direct save and the delete refuse + * that request since #23165; the draft save now refuses it the same way, with + * the same `409 RESOURCE_CONFLICT` and the same `?package=` remedy. + * + * Both orders are pinned, so a fix that merely flips which package wins + * cannot pass: the draft is refused under both. + */ +describe('[#23297] an item TWO packages hold: a DRAFT save naming neither is refused', () => { + const ORDERS = [ + { order: `${WRITABLE} saved first`, packages: [WRITABLE, OTHER_WRITABLE] }, + { order: `${OTHER_WRITABLE} saved first`, packages: [OTHER_WRITABLE, WRITABLE] }, + ] as const; + + for (const { order, packages } of ORDERS) { + it(`409 RESOURCE_CONFLICT naming both packages, and no draft is written (${order})`, async () => { + const name = 'shared_draft_save_app'; + const { protocol, rows, metadataWrites } = makeProtocol(); + for (const packageId of packages) { + await protocol.saveMetaItem({ type: 'app', name, item: app(name, { label: `Active ${packageId}` }), packageId }); + } + const before = metadataWrites().length; + + const err = await refusalOf(() => protocol.saveMetaItem({ + type: 'app', name, item: app(name, { label: 'Pending' }), mode: 'draft', + })); + + const envelope = resolveThrownHttpError(err); + expect(envelope.status).toBe(409); + expect(envelope.code).toBe('RESOURCE_CONFLICT'); + expect(err.message).toContain(`'${WRITABLE}'`); + expect(err.message).toContain(`'${OTHER_WRITABLE}'`); + expect(err.message).toContain('?package='); + expect(rows('app', name, 'draft')).toEqual([]); + expect(metadataWrites()).toHaveLength(before); + }); + } + + it('naming one of them saves that package\'s draft, and the other package has none', async () => { + const name = 'shared_draft_save_app'; + const { protocol, rows } = makeProtocol(); + for (const packageId of [WRITABLE, OTHER_WRITABLE]) { + await protocol.saveMetaItem({ type: 'app', name, item: app(name, { label: `Active ${packageId}` }), packageId }); + } + + const saved = await protocol.saveMetaItem({ + type: 'app', name, item: app(name, { label: 'Pending' }), mode: 'draft', packageId: OTHER_WRITABLE, + }); + + expect(saved.state).toBe('draft'); + expect(rows('app', name, 'draft').map((r) => r.package_id ?? null)).toEqual([OTHER_WRITABLE]); + }); +}); + +/** + * [#23297] CONTROL: with ONE package the draft still inherits it (#11087), and + * now whichever row the store returns first. A package-less active row beside + * the package's is not a second package (#23165's rule for the direct save): + * the draft goes into the package. Before, the inheritance read one active row, + * so with the package-less row first in storage the draft stayed package-less. + */ +describe('[#23297] CONTROL: one package and a package-less active row — the draft inherits the package in either storage order', () => { + for (const residueFirst of [false, true]) { + it(`the package-less row stored ${residueFirst ? 'first' : 'second'}`, async () => { + const { protocol, rows, seedRow } = makeProtocol(); + const name = 'residue_draft_app'; + const residue = () => seedRow({ + type: 'app', name, package_id: null, metadata: JSON.stringify(app(name, { active: false })), checksum: 'sha-residue', + }); + if (residueFirst) residue(); + await protocol.saveMetaItem({ type: 'app', name, item: app(name), packageId: WRITABLE }); + if (!residueFirst) residue(); + + const saved = await protocol.saveMetaItem({ type: 'app', name, item: app(name, { label: 'Pending' }), mode: 'draft' }); + + expect(saved.state).toBe('draft'); + expect(rows('app', name, 'draft').map((r) => r.package_id ?? null)).toEqual([WRITABLE]); + }); + } +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 16c97de7c53..bc08726fd25 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -55,6 +55,7 @@ import { envWritableMetadataTypes, historyRowsOfMember, packageScopedRowWhere, + packagelessAddressAmbiguityRefusal, resetEnvWritableMetadataTypes, type SysMetadataEngine, } from './sys-metadata-repository.js'; @@ -1181,51 +1182,6 @@ function authoredLockFamilyRefusal( return err; } -/** - * [#23165, ADR-0048] The refusal of a direct save, or a delete, that names no - * package, of an item whose rows at the address are bound to two or more - * packages ({@link ObjectStackProtocolImplementation.packagelessAddress}). - * - * A `sys_metadata` row is keyed `(org, type, name, package_id)`, so such a - * write has no one row to write or remove. Before this refusal the save wrote - * a package-less row beside the packages' rows and answered `200`, for a row - * the item read does not serve, and the delete removed whichever row the - * package-agnostic `findOne` met first and answered "it no longer exists" - * while another package's row stayed served. Choosing one of the packages is - * that driver-order coin toss (#8907). The remedy is the request's: name the - * package, which is the address every door of the item takes (`?package=`, - * ADR-0048). - * - * `RESOURCE_CONFLICT` / 409, no code minted: the standard catalog's member for - * a refusal that comes from environment state rather than the body (the same - * request is accepted once it names its package), and the code this - * package already answers when a name is held by more than one owner - * (`importUnmanagedPackage`). The prescription first, because a 4xx message - * crosses the REST boundary bounded at 500 characters with its tail - * truncated; the candidates follow, sorted, so the message is the same on - * every driver. - */ -function packagelessAddressAmbiguityRefusal( - verb: 'save' | 'delete', - type: string, - name: string, - packageIds: readonly string[], -): Error & { code: 'RESOURCE_CONFLICT'; status: 409 } { - const shown = packageIds.slice(0, 5).map((id) => `'${id}'`).join(', '); - const more = packageIds.length > 5 ? ` and ${packageIds.length - 5} more` : ''; - const [nothing, single] = verb === 'save' - ? ['Nothing was written.', 'no single row to write'] - : ['Nothing was deleted.', 'no single row to remove']; - const err = new Error( - `Name the package this ${verb} is for, then send it again: on the REST door add ?package= with the package id, ` - + `in process pass packageId. ${nothing} ${type} '${name}' belongs to ${packageIds.length} packages ` - + `(${shown}${more}), and a ${verb} that names none has ${single}.`, - ) as Error & { code: 'RESOURCE_CONFLICT'; status: 409 }; - err.code = 'RESOURCE_CONFLICT'; - err.status = 409; - return err; -} - /** * [#23165] The rows of an item a delete leaves stored at its address, in * words, for the delete receipt: its rows bound to other packages (sorted, as @@ -18616,11 +18572,20 @@ export class ObjectStackProtocolImplementation implements private async assertLockAllowsDelete(args: { type: string; name: string; + /** + * [#23297] The package the delete names (ADR-0048 `?package=`), handed + * to the gate as the save's is ({@link lockWriteRefusal}): part of the + * item's address. It never narrows the rows or the shipping packages + * the lock is resolved over (#21761, #21803: the strictest of them + * binds, so a delete naming a package is refused whenever the save + * naming it is); it picks whose prose the refusal carries. + */ + packageId?: string; actor?: string; source?: string; requestId?: string; }): Promise { - const state = await this.getEffectiveLock(args.type, args.name); + const state = await this.getEffectiveLock(args.type, args.name, args.packageId); const refusal = evaluateLockForDelete(state.lock); if (!refusal) return null; const reason = state.lockReason ?? refusal.reason; @@ -18830,8 +18795,9 @@ export class ObjectStackProtocolImplementation implements * one package's row of an item two packages hold, or the package-less row * beside a package's, and claimed the item gone. The draft save has * resolved its address from the item's active row since #11087 - * (`SysMetadataRepository.resolveWriteHead`, unchanged); the direct save - * and the delete did not, and this is that half. + * (`SysMetadataRepository.resolveWriteHead`, which [#23297] reads every + * active row and refuses two packages' with the same refusal); the direct + * save and the delete did not, and this is that half. * * The rows are the ones the item read chooses among * ({@link findServedOverlayRow}): the environment's (ADR-0131 D6), the @@ -20429,6 +20395,87 @@ export class ObjectStackProtocolImplementation implements healed = registry.removeRuntimeShadow(type, name, memberKey) || healed; } } + + // ── [#23297, ADR-0048] A ROW OF THE ITEM IS STILL STORED ── + // + // The tiers below conclude what is served next from the layers + // UNDER the deleted row (a packaged artifact, a MetadataService + // baseline, or nothing) and on "nothing" retire the entry. That + // presumed the deleted row was the item's only row. A delete names + // its package (#23165), so an item two packages hold keeps the + // other package's row stored and served by the item read and the + // list, while tier 3 retired the registry entry: the + // registry-backed doors (`getObject`, which data CRUD dispatches + // on, and the registry lookups) answered "not registered" until + // the next boot or save. Measured on a booted showcase: after + // `DELETE /api/v1/meta/object/x?package=P` with Q's row remaining, + // `GET /api/v1/meta/object/x` served Q's row and + // `GET /api/v1/data/x` answered 404 `OBJECT_NOT_FOUND`. + // + // So the store is asked first. A row still stored at the address + // (the environment's active rows, the canonical type, this member, + // every package) is what is served next: it is registered again + // the way a write makes a row live ({@link applyRegistryWriteThrough}), + // in the order the boot hydration ({@link loadMetaFromDb}) reads + // and with its sealed-object skip, and nothing is retired. Tier 1 + // above has already lifted the deleted row's shadow off a packaged + // artifact, so a remaining row the hydration declines (#22057) + // leaves the artifact served, as a boot does. + // + // An `object` has ONE owner per name in the registry (ADR-0029 + // D3): a second package's runtime-authored row never registered + // (its write-through was refused at save), so the owner can be the + // deleted row's package. That owner is unregistered first, only + // for a name no code package ships (the tier-3 limb's own guard), + // so the remaining row can take the name. An extender keeps it + // registered, said as tier 3 says it. + // + // An outage is not an absence: a failed read stops the walk with + // nothing retired, as a degraded baseline read does below. An + // unprovisioned store holds no row. + // The canonical key the repository stores rows under, and the one + // the write-through registers under (#8862, #9111). + const canonical = canonicalMetaType(type); + let storedRows: Array> | null | undefined; + try { + // [#21911] The explicit system opt-in — see findServedOverlayRow. + storedRows = await this.engine.find('sys_metadata', { + where: { type: canonical, name, state: 'active', organization_id: null, ...overlayMemberWhere(canonical, member) }, + context: { isSystem: true }, + }); + } catch (error) { + this.rethrowUnlessMetadataStoreUnprovisioned(error, 'sys_metadata'); + } + const remainingRows = canonical === 'object' && this.isUnservedSealedOverlay(canonical, name) + ? [] + : storedRows ?? []; + if (remainingRows.length > 0) { + if (canonical === 'object' && !this.isArtifactBacked(canonical, name)) { + const bindings = new Set(remainingRows.map((row) => boundPackageOf(row.package_id) ?? 'sys_metadata')); + const owner = typeof registry.getObjectOwner === 'function' ? registry.getObjectOwner(name) : undefined; + if (owner && !bindings.has(owner.packageId) && typeof registry.unregisterObject === 'function') { + try { + registry.unregisterObject(name); + } catch (err: any) { + console.warn( + `[Protocol] object '${name}' was deleted from sys_metadata but stays registered: ` + + `${err?.message ?? err}`, + ); + return; + } + } + } + for (const row of remainingRows) { + await this.applyRegistryWriteThrough({ + type: canonical, + name, + item: this.convertStoredItem(String(row.type ?? canonical), storedRowDocument(row)), + packageId: boundPackageOf(row.package_id) ?? null, + organizationId: null, + }); + } + return; + } if (healed) return; const baseline = await this.readItemFromMetadataService(type, name); @@ -28340,6 +28387,8 @@ export class ObjectStackProtocolImplementation implements const lockErr = await this.assertLockAllowsDelete({ type: request.type, name: request.name, + // [#23297] The delete's address carries its package, as the save's does. + ...(request.packageId ? { packageId: request.packageId } : {}), ...(request.actor ? { actor: request.actor } : {}), source: 'protocol.deleteMetaItem', }); diff --git a/packages/metadata-protocol/src/protocol.view-container-reset-expansions.test.ts b/packages/metadata-protocol/src/protocol.view-container-reset-expansions.test.ts index 55d3985a3fd..281e6153d16 100644 --- a/packages/metadata-protocol/src/protocol.view-container-reset-expansions.test.ts +++ b/packages/metadata-protocol/src/protocol.view-container-reset-expansions.test.ts @@ -461,8 +461,15 @@ describe('[#23149] a reset of a packaged view container heals the expansions its }); it('an organization-scoped heal touches none of the expansions; an environment-wide one after it heals them', async () => { - const { protocol, registry } = boot(); + const { protocol, registry, rows } = boot(); await publishContainerOverlay(protocol, xEdit('All Tasks (X)', true)); + // The heal runs after the delete that removed the row. [#23297] Since a + // row still stored is what the heal registers again (another package's + // row after a delete naming one package), the fixture removes the row + // the reset removes, rather than healing over a row that is still served. + for (const [key, row] of rows) { + if (row.type === 'view' && row.name === OBJ && row.state === 'active') rows.delete(key); + } const heal = (organizationId: string | null) => (protocol as any).restoreArtifactRegistryView('view', OBJ, organizationId) as Promise; diff --git a/packages/metadata-protocol/src/sys-metadata-repository.draft-package-inherit.test.ts b/packages/metadata-protocol/src/sys-metadata-repository.draft-package-inherit.test.ts index cb31f2fc9eb..aa729773a7e 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.draft-package-inherit.test.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.draft-package-inherit.test.ts @@ -30,6 +30,7 @@ import { describe, it, expect } from 'vitest'; // The engine-double contract gate: a fake looser than ObjectQL's own verb // dispatch is how #4434 shipped a dead route with its suite green. import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate, ConflictError } from '@objectstack/metadata-core'; +import { resolveThrownHttpError } from '@objectstack/types'; import { SysMetadataRepository } from './sys-metadata-repository.js'; interface Row { @@ -256,3 +257,68 @@ describe('SysMetadataRepository.headAt — the row put locks against (#22128)', expect(await repo.headAt(REF, { state: 'draft' })).toEqual({ head: null, packageId: null }); }); }); + +/** + * [#23297, ADR-0048] The inheritance reads EVERY active row of the address, + * never the first one the store returns. + * + * Bound to two packages, a package-less draft has no single package to go + * into: `put` refuses it `409 RESOURCE_CONFLICT` with the `?package=` remedy + * (the refusal the direct save and the delete answer since #23165) and writes + * nothing, and `headAt` names the package-unbound draft key, never a package + * picked by storage order. Bound to one package, with a package-less active + * row beside it, the draft inherits the package in either storage order. + */ +describe('[#23297] SysMetadataRepository draft inheritance reads every active row', () => { + const activeRow = (pkg: string | null, checksum: string) => ({ + type: REF.type, name: REF.name, organization_id: null, state: 'active', + package_id: pkg, metadata: '{"label":"Member"}', checksum, version: 1, + }); + + for (const order of [['app.k9qk', 'app.other'], ['app.other', 'app.k9qk']] as const) { + it(`two packages (${order.join(' stored before ')}): put refuses 409 RESOURCE_CONFLICT and writes nothing`, async () => { + const engine = makeFakeEngine(order.map((pkg, i) => activeRow(pkg, `sha-${i}`))); + const repo = makeRepo(engine); + const rowsBefore = engine.rows.length; + + const err = await repo + .put(REF, { label: 'Member', description: 'pending' }, { parentVersion: null, actor: 't', state: 'draft' as const }) + .then(() => null, (e: unknown) => e as Error & { code?: string; status?: number }); + + expect(err).not.toBeNull(); + const envelope = resolveThrownHttpError(err); + expect(envelope.status).toBe(409); + expect(envelope.code).toBe('RESOURCE_CONFLICT'); + expect(err!.message).toContain("'app.k9qk', 'app.other'"); + expect(err!.message).toContain('?package='); + expect(engine.rows).toHaveLength(rowsBefore); + expect(engine.history).toHaveLength(0); + }); + + it(`two packages (${order.join(' stored before ')}): headAt names the package-unbound draft key, no package`, async () => { + const engine = makeFakeEngine(order.map((pkg, i) => activeRow(pkg, `sha-${i}`))); + const repo = makeRepo(engine); + expect(await repo.headAt(REF, { state: 'draft', packageId: null })).toEqual({ head: null, packageId: null }); + }); + } + + it('two packages: a draft naming one of them is its own address and saves', async () => { + const engine = makeFakeEngine([activeRow('app.k9qk', 'sha-0'), activeRow('app.other', 'sha-1')]); + const repo = makeRepo(engine); + await repo.put(REF, { label: 'pending' }, { parentVersion: null, actor: 't', state: 'draft' as const, packageId: 'app.other' }); + expect(engine.rows.filter((r) => r.state === 'draft').map((r) => r.package_id)).toEqual(['app.other']); + }); + + for (const residueFirst of [true, false]) { + it(`one package and a package-less active row (${residueFirst ? 'package-less stored first' : 'package stored first'}): the draft inherits the package`, async () => { + const rows = residueFirst + ? [activeRow(null, 'sha-residue'), activeRow('app.k9qk', 'sha-active')] + : [activeRow('app.k9qk', 'sha-active'), activeRow(null, 'sha-residue')]; + const engine = makeFakeEngine(rows); + const repo = makeRepo(engine); + await repo.put(REF, { label: 'pending' }, { parentVersion: null, actor: 't', state: 'draft' as const }); + expect(engine.rows.filter((r) => r.state === 'draft').map((r) => r.package_id)).toEqual(['app.k9qk']); + expect((await repo.headAt(REF, { state: 'draft', packageId: null })).packageId).toBe('app.k9qk'); + }); + } +}); diff --git a/packages/metadata-protocol/src/sys-metadata-repository.ts b/packages/metadata-protocol/src/sys-metadata-repository.ts index 27d806dc580..b4d901f7ef0 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.ts @@ -530,6 +530,61 @@ export function packageScopedRowWhere( return where; } +/** + * [#23165, ADR-0048] The refusal of a save, or a delete, that names no + * package, of an item whose rows at the address are bound to two or more + * packages: the direct save and the delete, whose address the protocol + * resolves (`ObjectStackProtocolImplementation.packagelessAddress` in + * `protocol.ts`), and [#23297] the draft save, whose address + * {@link SysMetadataRepository.put} resolves (the inheritance in + * `resolveWriteHead`). One sentence for the three, so the doors cannot answer + * the same request in different words. + * + * A `sys_metadata` row is keyed `(org, type, name, package_id)`, so such a + * write has no one row to write or remove. Before this refusal the save wrote + * a package-less row beside the packages' rows and answered `200`, for a row + * the item read does not serve, and the delete removed whichever row the + * package-agnostic `findOne` met first and answered "it no longer exists" + * while another package's row stayed served; the draft save went into the + * package of whichever active row that `findOne` met first. Choosing one of + * the packages is that driver-order coin toss (#8907). The remedy is the + * request's: name the package, which is the address every door of the item + * takes (`?package=`, ADR-0048). + * + * `RESOURCE_CONFLICT` / 409, no code minted: the standard catalog's member for + * a refusal that comes from environment state rather than the body (the same + * request is accepted once it names its package), and the code this + * package already answers when a name is held by more than one owner + * (`importUnmanagedPackage`). The prescription first, because a 4xx message + * crosses the REST boundary bounded at 500 characters with its tail + * truncated; the candidates follow, sorted, so the message is the same on + * every driver. + * + * Deliberately NOT exported from the package entry: a shared refusal between + * this module and `protocol.ts`, as {@link packageScopedRowWhere} is a shared + * predicate. + */ +export function packagelessAddressAmbiguityRefusal( + verb: 'save' | 'delete', + type: string, + name: string, + packageIds: readonly string[], +): Error & { code: 'RESOURCE_CONFLICT'; status: 409 } { + const shown = packageIds.slice(0, 5).map((id) => `'${id}'`).join(', '); + const more = packageIds.length > 5 ? ` and ${packageIds.length - 5} more` : ''; + const [nothing, single] = verb === 'save' + ? ['Nothing was written.', 'no single row to write'] + : ['Nothing was deleted.', 'no single row to remove']; + const err = new Error( + `Name the package this ${verb} is for, then send it again: on the REST door add ?package= with the package id, ` + + `in process pass packageId. ${nothing} ${type} '${name}' belongs to ${packageIds.length} packages ` + + `(${shown}${more}), and a ${verb} that names none has ${single}.`, + ) as Error & { code: 'RESOURCE_CONFLICT'; status: 409 }; + err.code = 'RESOURCE_CONFLICT'; + err.status = 409; + return err; +} + /** * One live `watch()` subscription, as a record rather than a bare listener. * @@ -733,6 +788,21 @@ export class SysMetadataRepository implements MetadataRepository { * ADR-0048), and with no active row — a brand-new item drafted first — * there is nothing to inherit and the package-less semantics stand. * + * [#23297] The binding inherited is read from EVERY active row of the + * address, never from the first row a package-agnostic `findOne` returns + * (a driver-order coin toss, #8907): + * + * - bound to one package (a package-less row beside it is not a second + * package, #23165's rule for the direct save): the draft inherits it, + * whatever order the store returns the rows in; + * - bound to two or more: the draft has no single package to go into, so + * the resolution names no package and reports the bindings in + * `ambiguousPackages`. {@link put} refuses the write + * ({@link packagelessAddressAmbiguityRefusal}, the refusal the direct save + * and the delete answer); {@link headAt} answers the package-unbound + * draft key, so a read's version token never names a package picked by + * storage order. + * * [#22877] A write addresses ONE bundle member of a type whose identity is * a pair: `member` is that member as {@link OVERLAY_MEMBER_COLUMN} stores * it (`null` the canonical one), and every lookup below — the inherited @@ -747,9 +817,10 @@ export class SysMetadataRepository implements MetadataRepository { packageId: string | null | undefined, ctx?: Record, member?: string | null, - ): Promise<{ row: any | null; targetPackageId: string | null }> { + ): Promise<{ row: any | null; targetPackageId: string | null; ambiguousPackages?: readonly string[] }> { const context = { ...ctx, isSystem: true }; let targetPackageId: string | null = packageId ?? null; + let ambiguousPackages: readonly string[] | undefined; if (state === 'draft' && packageId == null) { // The overlaid base is resolved with the SAME two-scope reach the draft // list applies (`listDrafts`' $or contract): an org-scoped caller's @@ -772,9 +843,16 @@ export class SysMetadataRepository implements MetadataRepository { } else { activeWhere.organization_id = null; } - const activeRow = await this.engine.findOne('sys_metadata', { where: activeWhere, context }); - const activePkg = (activeRow as { package_id?: string | null } | null)?.package_id ?? null; - if (activePkg) targetPackageId = activePkg; + // [#23297] Every active row, never the first one: see this method's + // header. A failed read propagates (the caller's write fails), as the + // single-row read did. + const activeRows = await this.engine.find('sys_metadata', { where: activeWhere, context }); + const bindings = new Set(); + for (const activeRow of (activeRows ?? []) as Array<{ package_id?: unknown }>) { + if (typeof activeRow?.package_id === 'string' && activeRow.package_id !== '') bindings.add(activeRow.package_id); + } + if (bindings.size === 1) targetPackageId = [...bindings][0] ?? null; + else if (bindings.size > 1) ambiguousPackages = [...bindings].sort(); } let row = await this.engine.findOne('sys_metadata', { where: this.whereFor(ref, state, targetPackageId, member), @@ -792,7 +870,7 @@ export class SysMetadataRepository implements MetadataRepository { context, }); } - return { row: row ?? null, targetPackageId }; + return { row: row ?? null, targetPackageId, ...(ambiguousPackages ? { ambiguousPackages } : {}) }; } /** @@ -874,7 +952,15 @@ export class SysMetadataRepository implements MetadataRepository { // create, from THE write-address resolution ({@link resolveWriteHead}): // the one {@link headAt} answers too, so the head a caller was served // for this address is the head this lock judges. - const { row: existing, targetPackageId } = await this.resolveWriteHead(ref, state, opts.packageId, ctx, member); + const { row: existing, targetPackageId, ambiguousPackages } = await this.resolveWriteHead( + ref, state, opts.packageId, ctx, member, + ); + // [#23297] A draft that names no package, of an item whose active rows + // are bound to two or more packages, has no single row to write: refused + // before anything is written, as the direct save is (#23165). + if (ambiguousPackages) { + throw packagelessAddressAmbiguityRefusal('save', ref.type, ref.name, ambiguousPackages); + } // The row's stored stamp, as written: the parent link this write records // (`previous_checksum`, the event's `parentHash`) and the no-op check's // input. `null` for a row stored without one. diff --git a/packages/objectql/src/protocol-delete-object-registry-heal.test.ts b/packages/objectql/src/protocol-delete-object-registry-heal.test.ts index 32688bcaed1..a91912a1aef 100644 --- a/packages/objectql/src/protocol-delete-object-registry-heal.test.ts +++ b/packages/objectql/src/protocol-delete-object-registry-heal.test.ts @@ -415,3 +415,94 @@ describe('#6808 — deleteMetaItem stops BOTH registry exits serving a deleted o expect(plainKeyEntry(registry, 'myapp_invoice')).toBe(false); }); }); + +/** + * [#23297, ADR-0048] A delete naming ONE package, of an item two packages + * hold, leaves the other package's row stored and served by the store-backed + * doors (the item read, the list). The registry heal ran its tiers as if the + * deleted row were the whole item: no packaged artifact, no MetadataService + * baseline, so it retired the registry entry. For an `object` that is the + * entry the data plane dispatches on, so data CRUD answered `OBJECT_NOT_FOUND` + * for an object whose row the store still served, until the next boot or save. + * Measured on a booted showcase before the fix: after `DELETE + * /api/v1/meta/object/x?package=P`, `GET /api/v1/meta/object/x` answered 200 + * with Q's row while `GET` and `POST /api/v1/data/x` answered 404. + * + * The heal now asks the store first: when a row of the item is still stored, + * that row is what is served next, so it is registered again (the boot + * hydration's order) and nothing is retired. + * + * Both save orders: the registry holds ONE owner per object name, so the + * second package's write-through is refused at save time, and which package + * the registry holds before the delete depends on the order. + */ +describe('[#23297] a delete naming one package, of an object two packages hold, keeps the other package\'s row registered', () => { + const P = 'app.pkg_p'; + const Q = 'app.pkg_q'; + const bodyOf = (name: string, label: string) => ({ ...invoiceBody(name), label }); + + async function twoPackageObject(name: string, order: readonly string[], controlPlane: boolean) { + const harness = makeHarness({ controlPlane }); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + try { + for (const packageId of order) { + await harness.protocol.saveMetaItem({ type: 'object', name, packageId, item: bodyOf(name, `Invoice ${packageId}`) }); + } + } finally { + warn.mockRestore(); + } + expect(storedRows(harness.rows, name).map((r) => r.package_id).sort()).toEqual([P, Q].sort()); + return harness; + } + + for (const controlPlane of [false, true]) { + for (const order of [[P, Q], [Q, P]] as const) { + it(`${controlPlane ? 'control-plane' : 'tenant'} kernel, saved ${order.join(' then ')}: getObject, getItem and data CRUD serve Q's row after DELETE ?package=P`, async () => { + const name = 'shared_invoice'; + const { protocol, registry, rows, dataRows } = await twoPackageObject(name, order, controlPlane); + + const res = await protocol.deleteMetaItem({ type: 'object', name, packageId: P }); + + expect(res.success).toBe(true); + expect(storedRows(rows, name).map((r) => r.package_id)).toEqual([Q]); + expect((registry.getObject(name) as any)?.label).toBe(`Invoice ${Q}`); + expect((registry.getItem('object', name) as any)?.label).toBe(`Invoice ${Q}`); + expect(plainKeyEntry(registry, name)).toBe(true); + const created = await protocol.createData({ object: name, data: { name: 'INV-Q', amount: 1 } }); + expect(created.id).toBeTruthy(); + expect(dataRows.filter((r) => r.object === name)).toHaveLength(1); + }); + } + } + + it('CONTROL: deleting the last remaining row still retires the object (OBJECT_NOT_FOUND/404 on data CRUD)', async () => { + const name = 'shared_invoice'; + const { protocol, registry, rows } = await twoPackageObject(name, [P, Q], false); + await protocol.deleteMetaItem({ type: 'object', name, packageId: P }); + + await protocol.deleteMetaItem({ type: 'object', name, packageId: Q }); + + expect(storedRows(rows, name)).toHaveLength(0); + expect(registry.getObject(name)).toBeUndefined(); + expect(registry.getItem('object', name)).toBeUndefined(); + const err = await protocol + .createData({ object: name, data: { name: 'INV-X', amount: 1 } }) + .then(() => null, (e: any) => e); + expect(err?.code).toBe('OBJECT_NOT_FOUND'); + expect(err?.status).toBe(404); + }); + + it('a non-object item on a control-plane kernel: the registry serves the remaining package\'s row, not nothing', async () => { + const { protocol, registry, rows } = makeHarness({ controlPlane: true }); + const name = 'shared_dash'; + for (const packageId of [P, Q]) { + await protocol.saveMetaItem({ type: 'dashboard', name, packageId, item: { name, label: `Dash ${packageId}`, widgets: [] } }); + } + expect(storedRows(rows, name).map((r) => r.package_id).sort()).toEqual([P, Q].sort()); + + await protocol.deleteMetaItem({ type: 'dashboard', name, packageId: P }); + + expect(storedRows(rows, name).map((r) => r.package_id)).toEqual([Q]); + expect((registry.getItem('dashboard', name) as any)?.label).toBe(`Dash ${Q}`); + }); +}); diff --git a/packages/objectql/src/protocol-lock-artifact-package-axis.test.ts b/packages/objectql/src/protocol-lock-artifact-package-axis.test.ts index afd12ad93cc..46b5404e429 100644 --- a/packages/objectql/src/protocol-lock-artifact-package-axis.test.ts +++ b/packages/objectql/src/protocol-lock-artifact-package-axis.test.ts @@ -170,3 +170,61 @@ describe('[#21803] two installed packages ship one view: the real SchemaRegistry } }); }); + +/** + * [#23297, ADR-0010 §3.3, ADR-0048] The delete door's `_lock` gate takes the + * package the delete names (`?package=`, `packageId`), as the save gate does + * (#21761). What that address decides is the save gate's, and no more: + * + * 4. The verdict does not narrow to the named package. A ships no lock and B + * ships `'full'`: a delete naming A is refused `ITEM_LOCKED`, exactly as + * the save naming A is and as both reads naming A report. The lock is the + * strictest of every shipping package's answer (#21803); a door that + * admitted the delete would be looser than the read. + * 5. The prose follows the address. A and B both ship `'full'`, each with its + * own reason: the refusal of a delete naming a package carries that + * package's reason, as the save's does. Before, the delete handed the gate + * no package and carried the reason of the package whose id sorts first, + * whichever package the delete named. + */ +describe('[#23297] a delete naming a package: the save gate\'s verdict, and its prose', () => { + const refusalOf = async (protocol: ObjectStackProtocolImplementation, name: string, packageId: string, verb: 'save' | 'delete') => { + const err = await settle(verb === 'save' + ? protocol.saveMetaItem({ + type: 'view', name, item: { name, label: 'edited', object: 'account', viewKind: 'list' }, packageId, + }) + : protocol.deleteMetaItem({ type: 'view', name, packageId })); + return err ? { code: err.code, status: err.status, lock: err.lock, lockReason: err.lockReason } : 'admitted'; + }; + + describe('4. A ships no lock, B ships \'full\': the delete naming A is refused as the save naming A is', () => { + const arrangement: Arrangement = { name: 'v_del_a', locks: { [A]: undefined, [B]: 'full' } }; + for (const { order, packages } of ORDERS) { + it(`${order}`, async () => { + const protocol = protocolFor(arrangement, packages); + const save = await refusalOf(protocol, arrangement.name, A, 'save'); + const remove = await refusalOf(protocol, arrangement.name, A, 'delete'); + expect(save).toMatchObject(REFUSED_FULL); + expect(remove).toMatchObject(REFUSED_FULL); + expect(remove).toEqual(save); + const read: any = await protocol.getMetaItem({ type: 'view', name: arrangement.name, packageId: A }); + expect({ lock: read.lock, editable: read.editable, deletable: read.deletable }).toEqual(LOCKED_FULL); + }); + } + }); + + describe('5. A and B both ship \'full\', each with its own reason: the delete carries the named package\'s reason', () => { + const arrangement: Arrangement = { name: 'v_del_prose', locks: { [A]: 'full', [B]: 'full' } }; + for (const { order, packages } of ORDERS) { + for (const packageId of [A, B]) { + it(`${order} · delete naming ${packageId}`, async () => { + const protocol = protocolFor(arrangement, packages); + const save = await refusalOf(protocol, arrangement.name, packageId, 'save'); + const remove = await refusalOf(protocol, arrangement.name, packageId, 'delete'); + expect(remove).toMatchObject({ ...REFUSED_FULL, lockReason: `Packaged lock of ${packageId}.` }); + expect(remove).toEqual(save); + }); + } + } + }); +});