diff --git a/.changeset/23657-draft-save-one-package-draft.md b/.changeset/23657-draft-save-one-package-draft.md new file mode 100644 index 00000000000..40f93fd9491 --- /dev/null +++ b/.changeset/23657-draft-save-one-package-draft.md @@ -0,0 +1,38 @@ +--- +'@objectstack/metadata-protocol': minor +'@objectstack/spec': patch +--- + +fix(metadata-protocol)!: a draft save that names no package, of an item never published into a package, writes the draft the publish and the discard naming none act on, and is refused `409 RESOURCE_CONFLICT` where they are; the draft discard's receipt names the package (ADR-0048, ADR-0070) + +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 an item can hold a draft per package. Over an item never published into a package (no active row bound to one), the per-item publish (`POST /api/v1/meta/TYPE/NAME/publish`) and the draft discard (`DELETE /api/v1/meta/TYPE/NAME?state=draft`) that name no package act on the package-unbound draft, else on the one package's draft, and are refused `409 RESOURCE_CONFLICT` when two or more packages hold drafts. The draft save naming no package did not take that address. + +**FROM.** Over such an item, `PUT /api/v1/meta/TYPE/NAME?mode=draft` with no `?package=`: + +- With the only draft bound to package `Q` (saved with `?mode=draft&package=Q`), the save wrote a package-unbound draft beside `Q`'s and answered `200` ("Saved … (env-wide, state=draft)"). The drafts became `[package-unbound, Q]`, and from then on the publish and the discard naming no package acted on the package-unbound one, while `Q`'s draft stayed pending. +- With drafts bound to two or more packages and no package-unbound draft, the save wrote a package-unbound draft beside them and answered `200`, while the publish and the discard naming no package were refused `409`. +- `DELETE …?state=draft` of a package's draft answered "Draft discarded — TYPE/NAME", without the package. + +**TO.** + +- With the only draft bound to `Q`, the save updates `Q`'s draft and its receipt names the package ("Saved view 'my_view' (env-wide, package=Q, state=draft)"). The strict draft read's `version` (`GET …?state=draft`) is that draft's, so a save pinned to it with `If-Match` is accepted; the publish and the discard naming no package act on the same draft. +- With drafts bound to two or more packages and no package-unbound draft, the save is refused `409 RESOURCE_CONFLICT` and writes nothing, as the publish and the discard are. The message names the packages and the remedy, `?package=`. +- A draft discard of a package's draft names the package: "Draft discarded (package=Q) — TYPE/NAME. [seq=N]". The discard of a package-unbound draft reads as before. + +**What follows from the save writing `Q`'s draft**, over an item never published into a package whose only draft is bound to `Q`, for a draft save naming no package: + +- FROM `If-Match` carrying `Q`'s draft token (the version `GET …?state=draft&package=Q` serves) refused `409 METADATA_CONFLICT`, TO accepted: that draft is the row the save writes, so the strict draft read naming no package now serves the same token. +- FROM `If-None-Match: *` (a first-write pin) creating a package-unbound draft and answering `200`, TO `409 METADATA_CONFLICT` with nothing written: `Q`'s draft is the row the save would overwrite. +- FROM a permission set whose body declares `packageId: Q` refused `400 VALIDATION_ERROR` (the owner binding of ADR-0086 D3, judged against a save "into no package"), TO accepted: the binding the write resolves is `Q`. A body declaring another package is still refused, and the refusal now names `Q`. + +`@objectstack/spec`: the `SaveMetaItemRequestSchema.packageId` description now states the draft save's address in the words the publish's and the discard's descriptions use: the draft of the one package the item's active rows are bound to, else its package-unbound draft; for an item never published into a package, the package-unbound draft, else the one package's draft; refused `409 RESOURCE_CONFLICT` when the active rows (or, with none and no package-unbound draft, the drafts) are bound to two or more packages. It said the save wrote "the env-local draft" whenever the active row was not bound to a package. No key, type, enum or default moves. + +**Unchanged.** A draft save that names its package (`?package=`, `packageId` in process). An item with an active row bound to a package, whose draft save naming no package still inherits that package. An item with a package-unbound draft, which a draft save naming no package still updates, whatever packages' drafts sit beside it. A brand-new item with no draft at all, whose first draft save naming no package writes the package-unbound draft. No stored row is rewritten or removed by the upgrade. + +**Remedy.** For an item two or more packages hold drafts of, send the draft save with `?package=` naming the package (`packageId` in process). A client that compares draft-discard receipt text should expect ` (package=ID)` after `Draft discarded` when the draft discarded is bound to a package. diff --git a/content/docs/references/api/protocol.mdx b/content/docs/references/api/protocol.mdx index 4db12b203f1..13b26951251 100644 --- a/content/docs/references/api/protocol.mdx +++ b/content/docs/references/api/protocol.mdx @@ -2742,7 +2742,7 @@ Installed package with runtime lifecycle state | **actor** | `string` | optional | Identity recorded on the write's history event (`recorded_by`, a lookup into `sys_user`) and audit row. On the REST door this is the request's authenticated identity (one producer) — never a caller-supplied header. Absent, the event is recorded actor-less (null), deliberately not attributed to "system". | | **force** | `boolean` | optional | Destructive-change acknowledgement (`?force=true` on the REST door): skips the safety diff that refuses an `object` save whose body drops fields or narrows types the stored item still carries (409 with the findings otherwise). Only `object` saves reach that diff, so the flag is inert for every other type. Absent = the guard runs. | | **mode** | `Enum<'draft' \| 'publish'>` | optional | Per-item lifecycle (ADR-0005 drafts): `draft` stages the body as a pending draft overlay (`?mode=draft` on the REST door; the publish door promotes it later); `publish` or ABSENT writes straight to the live `active` row — the legacy default, kept so callers that predate the draft/publish split keep working. Any value other than `draft` is read as `publish`. | -| **packageId** | `string \| null` | optional | ADR-0048 — the software package to bind the saved row to (`sys_metadata.package_id`; `?package=` on the REST door, where `all` and an empty value name no package). Set when authoring inside a Studio package workspace; a named read-only base package is refused. On create the row is stamped with this id; on update an existing binding is preserved, never silently re-bound. Absent or `null` names no package. An `active` save then writes the row of the one package the item's active rows are bound to, as a save naming that package does, is refused `409 RESOURCE_CONFLICT` when they are bound to two or more, and with none writes the env-local overlay (no package stamp); a `draft` save writes the draft of the package the item's active row is bound to when it is package-bound (a pending change stays in the package it edits), else the env-local draft. The row written is also the one the parent-version resolution reads: an unpinned save takes its version as the parent (last-write-wins), a pinned one is judged against it, and the item read at the same scope serves it as `version`. | +| **packageId** | `string \| null` | optional | ADR-0048 — the software package to bind the saved row to (`sys_metadata.package_id`; `?package=` on the REST door, where `all` and an empty value name no package). Set when authoring inside a Studio package workspace; a named read-only base package is refused. On create the row is stamped with this id; on update an existing binding is preserved, never silently re-bound. Absent or `null` names no package. An `active` save then writes the row of the one package the item's active rows are bound to, as a save naming that package does, is refused `409 RESOURCE_CONFLICT` when they are bound to two or more, and with none writes the env-local overlay (no package stamp); a `draft` save writes the draft of the one package the item's active rows are bound to (a pending change stays in the package it edits), else its package-unbound draft, adopted into that package — the draft the per-item publish naming no package promotes. For an item never published into a package it is the package-unbound draft, else the one package's draft, and with no draft at all the save creates the package-unbound draft. It is refused `409 RESOURCE_CONFLICT` when the active rows (or, with none and no package-unbound draft, the drafts) are bound to two or more packages, as that publish is (see `PublishMetaItemRequestSchema.packageId`). The row written is also the one the parent-version resolution reads: an unpinned save takes its version as the parent (last-write-wins), a pinned one is judged against it, and the item read at the same scope serves it as `version`. | | **writeFace** | `Enum<'package-duplicate' \| 'meta-envelope' \| 'meta-dispatch' \| 'external-import'>` | optional | Which write door a refusal is being rendered FOR — stated by the SERVER, never by a remote caller: every door builds this request field by field and never spreads a request body into it, so there is no path for a client to smuggle a face in, and a face arriving in a wire body is simply never read. Two refusals branch on it, for different questions: the 409 destructive-change remedy names the acknowledgement mechanism that actually exists on the refusing door (`?force=true` on the REST doors; the dispatcher, the duplicate-package door and the external-table import have none), and the 422 invalid-metadata message adapts to whether a structured `issues[]` channel reaches the consumer beside it. Absent = the conservative default wording. | diff --git a/packages/metadata-protocol/src/protocol.item-name-every-door.test.ts b/packages/metadata-protocol/src/protocol.item-name-every-door.test.ts index e433bc757d3..7b69d671b17 100644 --- a/packages/metadata-protocol/src/protocol.item-name-every-door.test.ts +++ b/packages/metadata-protocol/src/protocol.item-name-every-door.test.ts @@ -611,6 +611,23 @@ describe('[ADR-0086 D3] the save door refuses a permission set whose body `packa ]); }); + it('[#23657] an item never published into a package, whose only draft is BOUND\'s: a draft naming no package is judged against BOUND, the draft it writes', async () => { + // No active row, so the draft address is read from the drafts: the + // only draft is BOUND's, and a package-less draft save updates it. + // Before, the save wrote a package-unbound draft beside it, and an + // agreeing body was refused 400 for declaring a package "into no package". + const h = makeProtocol(); + await h.protocol.saveMetaItem({ type: 'permission', name: 'probe_member', item: perm('probe_member', BOUND), packageId: BOUND, mode: 'draft' }); + + const { error } = await attempt(() => h.protocol.saveMetaItem({ + type: 'permission', name: 'probe_member', item: perm('probe_member', OTHER), mode: 'draft', + })); + expectOwnerRefusal(error, OTHER, BOUND); + + await h.protocol.saveMetaItem({ type: 'permission', name: 'probe_member', item: { ...perm('probe_member', BOUND), label: 'Edited' }, mode: 'draft' }); + expect(storedSets(h)).toEqual([{ row: 'probe_member', state: 'draft', binding: BOUND, body: BOUND }]); + }); + it('a direct save that names no package is judged against the one package its address resolves to', async () => { // The item's only active row is bound to BOUND, so a package-less // direct save writes BOUND's row: an agreeing body passes, and a 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 079d0ef9e6d..505949d6c90 100644 --- a/packages/metadata-protocol/src/protocol.packageless-save-address.test.ts +++ b/packages/metadata-protocol/src/protocol.packageless-save-address.test.ts @@ -1022,7 +1022,9 @@ describe('[#23495] the draft doors naming no package act on the draft the draft await f.protocol.saveMetaItem({ type: 'app', name, item: app(name, { label: 'pending' }), packageId: P, mode: 'draft' }); } else { await f.protocol.saveMetaItem({ type: 'app', name, item: app(name, { label: 'pending' }), packageId: P, mode: 'draft' }); - await saveDraft(f, 'stale'); + // [#23657] Seeded, as an older build left it: a draft save naming no + // package over an item whose only draft is P's now updates P's draft. + f.seedRow({ type: 'app', name, state: 'draft', package_id: null, metadata: JSON.stringify(app(name, { label: 'stale' })), checksum: 'sha-stale' }); await f.protocol.saveMetaItem({ type: 'app', name, item: app(name, { label: 'live' }), packageId: P }); } return f; @@ -1180,7 +1182,9 @@ describe('[#23495] the draft doors naming no package act on the draft the draft const f = { ...makeProtocol(), name }; if (unboundFirst) await saveDraft(f, 'unbound'); await f.protocol.saveMetaItem({ type: 'app', name, item: app(name, { label: 'other' }), packageId: Q, mode: 'draft' }); - if (!unboundFirst) await saveDraft(f, 'unbound'); + // [#23657] Seeded, as an older build left it: a draft save naming + // no package over Q's only draft now updates Q's draft. + if (!unboundFirst) f.seedRow({ type: 'app', name, state: 'draft', package_id: null, metadata: JSON.stringify(app(name, { label: 'unbound' })), checksum: 'sha-unbound' }); return f; }; diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 700377269e0..7b25a896d02 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -65,6 +65,7 @@ import { packageScopedRowWhere, packagelessAddressAmbiguityRefusal, resetEnvWritableMetadataTypes, + unboundItemDraftAddress, type SysMetadataEngine, } from './sys-metadata-repository.js'; import { isOriginGatedType, managedItemSealedSentence } from './packaged-base-regime.js'; @@ -19135,10 +19136,12 @@ export class ObjectStackProtocolImplementation implements * with the refusal the draft save answers * ({@link packagelessAddressAmbiguityRefusal}, `409 RESOURCE_CONFLICT`). * - bound to none (an item never published into a package): the - * package-unbound draft, the one the draft save writes. With no such - * draft, the one package's draft when exactly one package holds a draft - * (the only draft the read naming no package can serve); two or more - * is `ambiguous`; none at all is `none`. + * package-unbound draft. With no such draft, the one package's draft + * when exactly one package holds a draft (the only draft the read + * naming no package can serve); two or more is `ambiguous`; none at + * all is `none`. [#23657] The draft save naming no package writes the + * same draft and is refused the same way: one rule, + * {@link unboundItemDraftAddress}, which `resolveWriteHead` asks too. * * Every row is read in full, never sampled: a package-agnostic `findOne` * over two drafts is a driver-order coin toss (#8907). The per-item publish @@ -19176,11 +19179,11 @@ export class ObjectStackProtocolImplementation implements if (drafts.unbound) return { kind: 'draft', key: null, into: own, drafts }; return { kind: 'none', key: own, into: own, drafts }; } - if (drafts.unbound) return { kind: 'draft', key: null, into: null, drafts }; - if (drafts.candidates.length > 1) return { kind: 'ambiguous', candidates: drafts.candidates }; - const only = drafts.candidates[0]; - if (only !== undefined) return { kind: 'draft', key: only, into: only, drafts }; - return { kind: 'none', key: null, into: null, drafts }; + // [#23657] Bound to none: the rule the draft save's address takes too + // ({@link unboundItemDraftAddress}, in `resolveWriteHead`). + const address = unboundItemDraftAddress(drafts); + if (address.kind === 'ambiguous') return address; + return { kind: address.kind, key: address.key, into: address.key, drafts }; } /** @@ -29794,8 +29797,13 @@ export class ObjectStackProtocolImplementation implements // package's row names the package, as the save receipt // does; a delete of the package-unbound row with nothing // left reads byte for byte as before. + // + // [#23657] The draft discard names the package of the + // draft it removed too, as the draft save's receipt names + // the package it wrote into; a package-unbound draft's + // discard reads byte for byte as before. message: (request.state === 'draft') - ? `Draft discarded — ${request.type}/${request.name}. [seq=${result.seq}]` + ? `Draft discarded${deletedPackage} — ${request.type}/${request.name}. [seq=${result.seq}]` : artifactBacked ? (remains ? `Customization overlay deleted${deletedPackage} — ${request.type}/${request.name} still carries another overlay: ${remainingRowsPhrase(remainingPackages, unboundRemains, 'row')}. [seq=${result.seq}]` 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 5b64c592d68..5cdc4818787 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 @@ -333,3 +333,72 @@ describe('[#23495] SysMetadataRepository.promoteDraft adoptInto', () => { expect(activeRows(engine)).toEqual([['app.k9qk', 'live'], [null, 'orphan']]); }); }); + +/** + * [#23657, ADR-0070 TL;DR 1] An item with no active row bound to a package: + * the draft address is read from the DRAFT rows (`unboundItemDraftAddress`, the + * rule the publish and the discard naming no package take). The package-unbound + * draft keeps it; else the one package's draft is the address, so `put` + * updates it and `headAt` serves it, never a package-unbound draft minted + * beside it; two or more packages' drafts is refused `409 RESOURCE_CONFLICT`. + */ +describe('[#23657] SysMetadataRepository draft address of an item never published into a package', () => { + const draftRow = (pkg: string | null, checksum: string) => ({ + type: REF.type, name: REF.name, state: 'draft', + package_id: pkg, metadata: '{"label":"Member"}', checksum, version: 1, + }); + + it('the only draft bound to one package: put updates it, and headAt serves it', async () => { + const engine = makeFakeEngine([draftRow('app.q', 'sha-q')]); + const repo = makeRepo(engine); + const head = await repo.headAt(REF, { state: 'draft', packageId: null }); + expect(head.packageId).toBe('app.q'); + expect(head.head?.hash).toBe('sha-q'); + // The parent the head named is the one put's lock accepts. + await repo.put(REF, { label: 'edited' }, { parentVersion: head.head!.hash, actor: 't', state: 'draft' as const }); + const drafts = engine.rows.filter((r) => r.state === 'draft'); + expect(drafts.map((r) => r.package_id)).toEqual(['app.q']); + expect(JSON.parse(String(drafts[0]!.metadata)).label).toBe('edited'); + }); + + for (const order of [['app.q', 'app.r'], ['app.r', 'app.q']] as const) { + it(`two packages' drafts (${order.join(' stored before ')}): put refuses 409 RESOURCE_CONFLICT and writes nothing`, async () => { + const engine = makeFakeEngine(order.map((pkg, i) => draftRow(pkg, `sha-${i}`))); + const repo = makeRepo(engine); + const err = await repo + .put(REF, { label: 'pending' }, { parentVersion: null, actor: 't', state: 'draft' as const }) + .then(() => null, (e: unknown) => e as Error); + expect(err).not.toBeNull(); + const envelope = resolveThrownHttpError(err); + expect(envelope.status).toBe(409); + expect(envelope.code).toBe('RESOURCE_CONFLICT'); + expect(err!.message).toContain("'app.q', 'app.r'"); + expect(engine.rows).toHaveLength(2); + expect(engine.history).toHaveLength(0); + expect(await repo.headAt(REF, { state: 'draft', packageId: null })).toEqual({ head: null, packageId: null }); + }); + } + + it('control: a package-unbound draft beside packages\' drafts keeps the address', async () => { + const engine = makeFakeEngine([draftRow('app.q', 'sha-q'), draftRow(null, 'sha-u'), draftRow('app.r', 'sha-r')]); + const repo = makeRepo(engine); + const head = await repo.headAt(REF, { state: 'draft', packageId: null }); + expect(head.packageId).toBeNull(); + expect(head.head?.hash).toBe('sha-u'); + await repo.put(REF, { label: 'edited' }, { parentVersion: 'sha-u', actor: 't', state: 'draft' as const }); + const unbound = engine.rows.filter((r) => r.state === 'draft' && (r.package_id ?? null) === null); + expect(unbound).toHaveLength(1); + expect(JSON.parse(String(unbound[0]!.metadata)).label).toBe('edited'); + expect(engine.rows.filter((r) => r.state === 'draft')).toHaveLength(3); + }); + + it('control: an active binding decides, whatever package another draft is bound to', async () => { + const engine = makeFakeEngine([ + { type: REF.type, name: REF.name, state: 'active', package_id: 'app.k9qk', metadata: '{"label":"Member"}', checksum: 'sha-a', version: 1 }, + draftRow('app.q', 'sha-q'), + ]); + const repo = makeRepo(engine); + await repo.put(REF, { label: 'edited' }, { parentVersion: null, actor: 't', state: 'draft' as const }); + expect(engine.rows.filter((r) => r.state === 'draft').map((r) => r.package_id).sort()).toEqual(['app.k9qk', 'app.q']); + }); +}); diff --git a/packages/metadata-protocol/src/sys-metadata-repository.ts b/packages/metadata-protocol/src/sys-metadata-repository.ts index 2a8558481c8..2f2030f9d20 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.ts @@ -584,6 +584,41 @@ export function packagelessAddressAmbiguityRefusal( return err; } +/** + * [#23657, ADR-0048, ADR-0070 TL;DR 1] THE draft address of an item never + * published into a package (no active row bound to one), from the bindings of + * its draft rows: the package-unbound draft when one is stored (ADR-0048 + * prefer-local); with none, the one package's draft when exactly one package + * holds a draft; two or more packages is `ambiguous`, refused with + * {@link packagelessAddressAmbiguityRefusal}; no draft at all is `none`, whose + * key is the package-unbound row a create writes. + * + * One rule for every draft door that names no package, so they cannot address + * different drafts: the draft save ({@link SysMetadataRepository.put}, through + * `resolveWriteHead`, and so `headAt` and the strict draft read's `version`), + * and the per-item publish and the draft discard + * (`ObjectStackProtocolImplementation.packagelessDraftAddress` in + * `protocol.ts`). The draft save used to write the package-unbound draft here + * whatever was stored, so over an item whose only draft is bound to `Q` it + * minted a second draft beside `Q`'s, while the publish and the discard acted + * on `Q`'s. + * + * `drafts` is every draft row of the address read in full, never sampled + * (#8907): `candidates` the distinct package bindings, sorted, `unbound` + * whether a package-unbound draft is among them. + * + * Deliberately NOT exported from the package entry, as + * {@link packagelessAddressAmbiguityRefusal} is not. + */ +export function unboundItemDraftAddress( + drafts: { candidates: readonly string[]; unbound: boolean }, +): { kind: 'ambiguous'; candidates: readonly string[] } | { kind: 'draft' | 'none'; key: string | null } { + if (drafts.unbound) return { kind: 'draft', key: null }; + if (drafts.candidates.length > 1) return { kind: 'ambiguous', candidates: drafts.candidates }; + const only = drafts.candidates[0]; + return only !== undefined ? { kind: 'draft', key: only } : { kind: 'none', key: null }; +} + /** * One live `watch()` subscription, as a record rather than a bare listener. * @@ -783,7 +818,8 @@ export class SysMetadataRepository implements MetadataRepository { * Inherit the overlaid ACTIVE row's binding instead. An explicit * `packageId` is untouched (a caller that states its base keeps it, * ADR-0048), and with no active row — a brand-new item drafted first — - * there is nothing to inherit and the package-less semantics stand. + * there is nothing to inherit from the active rows (the draft rows decide, + * [#23657] below). * * [#23297] The binding inherited is read from EVERY active row of the * address, never from the first row a package-agnostic `findOne` returns @@ -798,7 +834,14 @@ export class SysMetadataRepository implements MetadataRepository { * ({@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. + * storage order; + * - [#23657] bound to none (an item never published into a package): the + * address is read from the item's draft rows + * ({@link unboundItemDraftAddress}, the rule the per-item publish and the + * draft discard naming no package take): the package-unbound draft, else + * the one package's draft, else, with two or more packages' drafts, + * `ambiguousPackages`, refused by {@link put} as above. No draft at all + * keeps the package-less semantics of a brand-new item drafted first. * * [#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 @@ -844,6 +887,30 @@ export class SysMetadataRepository implements MetadataRepository { } if (bindings.size === 1) targetPackageId = [...bindings][0] ?? null; else if (bindings.size > 1) ambiguousPackages = [...bindings].sort(); + else { + // [#23657, ADR-0070 TL;DR 1] No active row bound to a package (an + // item never published into one): the address is read from the + // item's DRAFT rows, by the rule the per-item publish and the draft + // discard naming no package take ({@link unboundItemDraftAddress}). + // A package-unbound draft keeps the address; with none, the one + // package's draft is the address, never a package-unbound draft + // minted beside it; two or more packages' drafts is refused by + // {@link put}, as for two packages' active rows. + const draftWhere: Record = { ...activeWhere, state: 'draft' }; + const draftRows = await this.withoutLegacyOrganizationRows( + await this.engine.find('sys_metadata', { where: draftWhere, context }), + context, + ); + const draftBindings = new Set(); + let unbound = false; + for (const draftRow of (draftRows ?? []) as Array<{ package_id?: unknown }>) { + if (typeof draftRow?.package_id === 'string' && draftRow.package_id !== '') draftBindings.add(draftRow.package_id); + else unbound = true; + } + const address = unboundItemDraftAddress({ candidates: [...draftBindings].sort(), unbound }); + if (address.kind === 'ambiguous') ambiguousPackages = address.candidates; + else targetPackageId = address.key; + } } let row = await this.findEnvironmentRow({ where: this.whereFor(ref, state, targetPackageId, member), diff --git a/packages/rest/src/meta-draft-head-package-inheritance.test.ts b/packages/rest/src/meta-draft-head-package-inheritance.test.ts index 873490b044c..d663055de4c 100644 --- a/packages/rest/src/meta-draft-head-package-inheritance.test.ts +++ b/packages/rest/src/meta-draft-head-package-inheritance.test.ts @@ -524,3 +524,124 @@ describe('[#23495] the draft doors naming no package act on the draft the draft } }, 60_000); }); + +/** + * [#23657, ADR-0048, ADR-0070 TL;DR 1] An item never published into a package + * (no active row bound to one), whose drafts are bound to packages: the draft + * save naming no package takes the address the publish and the discard naming + * none take (#23495). With no package-unbound draft and exactly one package's + * draft, that draft is the address, so the save updates it instead of minting a + * package-unbound draft beside it; with two or more packages' drafts the save + * is refused `409 RESOURCE_CONFLICT` with the `?package=` remedy, as the + * publish and the discard are. Measured on these routes before the fix: the + * save answered `200` and the drafts became `[package-unbound, Q]`, after + * which the publish and the discard acted on the package-unbound one. + */ +describe('[#23657] an item never published into a package: the draft save naming no package takes the draft doors\' address', () => { + const Q = 'com.probe.q'; + const R = 'com.probe.r'; + + /** No active row; the only draft bound to `Q`. */ + const corner = async () => { + const h = await boot(); + ok(await h.save(view('pending'), { mode: 'draft', package: Q })); + return h; + }; + + it('the only draft bound to Q: the save updates Q\'s draft, and the read, the publish and the discard naming none act on it', async () => { + const s = await corner(); + const saved = ok(await s.save(view('saved'), { mode: 'draft' })); + expect(saved.message).toContain(`package=${Q}`); + expect(await s.stored('draft')).toEqual([{ label: 'saved', packageId: Q }]); + // The strict draft read serves that draft, with the version the save + // answered, which a second save pinned to it is accepted against. + const read = ok(await s.read({ state: 'draft' })); + expect(read.item.label).toBe('saved'); + expect(read.version).toMatch(KEYED); + expect(read.version).toBe(saved.version); + ok(await s.save(view('saved again'), { mode: 'draft' }, { 'if-match': read.version })); + expect(await s.stored('draft')).toEqual([{ label: 'saved again', packageId: Q }]); + + ok(await s.publish()); + expect(await s.stored('active')).toEqual([{ label: 'saved again', packageId: Q }]); + expect(await s.stored('draft')).toEqual([]); + + const d = await corner(); + ok(await d.save(view('saved'), { mode: 'draft' })); + const discarded = ok(await d.remove({ state: 'draft' })); + expect(discarded.reset).toBe(true); + expect(await d.stored('draft')).toEqual([]); + }, 60_000); + + it('the only draft bound to Q: a first-write pin (`If-None-Match: *`) is refused 409 METADATA_CONFLICT, because Q\'s draft is the row it writes', async () => { + const s = await corner(); + refusedConflict(await s.save(view('first?'), { mode: 'draft' }, { 'if-none-match': '*' })); + expect(await s.stored('draft')).toEqual([{ label: 'pending', packageId: Q }]); + }, 60_000); + + it('the discard receipt names the package of the draft it removed; a package-unbound draft\'s receipt reads as before', async () => { + const d = await corner(); + const bound = ok(await d.remove({ state: 'draft' })); + expect(bound.message).toContain(`(package=${Q})`); + + const u = await boot(); + ok(await u.save(view('plain'), { mode: 'draft' })); + const unbound = ok(await u.remove({ state: 'draft' })); + expect(unbound.message).toBe(`Draft discarded — view/case_grid. [seq=${unbound.seq}]`); + }, 60_000); + + it('drafts bound to two packages and no package-unbound draft: the save naming none is refused 409 RESOURCE_CONFLICT, as the publish and the discard are, and writes nothing', async () => { + const build = async () => { + const h = await boot(); + ok(await h.save(view('q'), { mode: 'draft', package: Q })); + ok(await h.save(view('r'), { mode: 'draft', package: R })); + return h; + }; + const both = [{ label: 'q', packageId: Q }, { label: 'r', packageId: R }]; + for (const door of ['save', 'publish', 'discard'] as const) { + const h = await build(); + const res = door === 'save' + ? await h.save(view('saved'), { mode: 'draft' }) + : door === 'publish' ? await h.publish() : await h.remove({ state: 'draft' }); + expect(res.statusCode, `${door}: ${JSON.stringify(res.body)}`).toBe(409); + expect(res.body.code, door).toBe('RESOURCE_CONFLICT'); + expect(res.body.error, door).toContain('?package='); + expect(res.body.error, door).toContain(R); + expect(await h.stored('draft'), door).toEqual(expect.arrayContaining(both)); + expect(await h.stored('draft'), door).toHaveLength(2); + expect(await h.stored('active'), door).toEqual([]); + } + // The remedy: naming the package saves that package's draft. + const named = await build(); + ok(await named.save(view('saved'), { mode: 'draft', package: R })); + expect(await named.stored('draft')).toEqual(expect.arrayContaining([{ label: 'q', packageId: Q }, { label: 'saved', packageId: R }])); + }, 60_000); + + it('control: an item with an active binding inherits the active row\'s package, whatever package another draft is bound to', async () => { + const h = await boot(); + ok(await h.save(view('live'), { package: PKG })); + ok(await h.save(view('pending'), { mode: 'draft', package: Q })); + const saved = ok(await h.save(view('saved'), { mode: 'draft' })); + expect(saved.message).toContain(`package=${PKG}`); + expect(await h.stored('draft')).toEqual(expect.arrayContaining([{ label: 'pending', packageId: Q }, { label: 'saved', packageId: PKG }])); + expect(await h.stored('draft')).toHaveLength(2); + }, 60_000); + + it('control: a package-unbound draft is the address whatever packages\' drafts sit beside it, as for the publish and the discard', async () => { + const only = await boot(); + ok(await only.save(view('plain'), { mode: 'draft' })); + const saved = ok(await only.save(view('plain 2'), { mode: 'draft' })); + expect(saved.message).not.toContain('package='); + expect(await only.stored('draft')).toEqual([{ label: 'plain 2', packageId: null }]); + + const beside = await boot(); + ok(await beside.save(view('plain'), { mode: 'draft' })); + ok(await beside.save(view('q'), { mode: 'draft', package: Q })); + ok(await beside.save(view('r'), { mode: 'draft', package: R })); + ok(await beside.save(view('plain 2'), { mode: 'draft' })); + expect(await beside.stored('draft')).toEqual(expect.arrayContaining([ + { label: 'plain 2', packageId: null }, { label: 'q', packageId: Q }, { label: 'r', packageId: R }, + ])); + expect(await beside.stored('draft')).toHaveLength(3); + }, 60_000); +}); diff --git a/packages/spec/src/api/protocol.zod.ts b/packages/spec/src/api/protocol.zod.ts index 419dc4ca61f..1cea389fcb6 100644 --- a/packages/spec/src/api/protocol.zod.ts +++ b/packages/spec/src/api/protocol.zod.ts @@ -845,10 +845,16 @@ export const SaveMetaItemRequestSchema = lazySchema(() => z.object({ + 'one package the item\'s active rows are bound to, as a save naming ' + 'that package does, is refused `409 RESOURCE_CONFLICT` when they are ' + 'bound to two or more, and with none writes the env-local overlay (no ' - + 'package stamp); a `draft` save writes the draft of ' - + 'the package the item\'s active row is bound to when it is ' - + 'package-bound (a pending change stays in the package it edits), else ' - + 'the env-local draft. The row written is also the one the parent-version ' + + 'package stamp); a `draft` save writes the draft of the one package ' + + 'the item\'s active rows are bound to (a pending change stays in the ' + + 'package it edits), else its package-unbound draft, adopted into that ' + + 'package — the draft the per-item publish naming no package promotes. ' + + 'For an item never published into a package it is the package-unbound ' + + 'draft, else the one package\'s draft, and with no draft at all the save ' + + 'creates the package-unbound draft. It is refused `409 RESOURCE_CONFLICT` ' + + 'when the active rows (or, with none and no package-unbound draft, the ' + + 'drafts) are bound to two or more packages, as that publish is (see ' + + '`PublishMetaItemRequestSchema.packageId`). The row written is also the one the parent-version ' + 'resolution reads: an unpinned save takes its version as the parent ' + '(last-write-wins), a pinned one is judged against it, and the item ' + 'read at the same scope serves it as `version`.',