From 21d1cbc1203ea5e74c899c36da951fe675eefde4 Mon Sep 17 00:00:00 2001 From: os-elon Date: Sat, 22 Aug 2026 03:43:49 +0000 Subject: [PATCH 1/3] fix(metadata-protocol): declare packageId on publishMetaItem's request type `publishMetaItem` declared `type / name / organizationId / actor / message / _skipSeedApply` and no `packageId`, while `POST /meta/:type/:name/publish ?package=PKG_ID` (#10063) states one on every HTTP-driven promotion that names a package. The value already flowed correctly - the request object is forwarded whole and `canonicalizeMetaRequestType` is a spread - but the binding was invisible to every typed caller and one destructuring refactor away from being dropped in silence. Also corrects three in-tree comments that claimed the per-item door "names no package", which stopped being true at #10063. The `draftForGate` / `rowToItem` warning those comments were really defending is preserved verbatim. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM --- ...tocol-publish-drafts-package-scope.test.ts | 90 +++++++++++++++++++ packages/metadata-protocol/src/protocol.ts | 87 +++++++++++++++--- 2 files changed, 163 insertions(+), 14 deletions(-) diff --git a/packages/metadata-protocol/src/protocol-publish-drafts-package-scope.test.ts b/packages/metadata-protocol/src/protocol-publish-drafts-package-scope.test.ts index d753623617..b80b0a6e1f 100644 --- a/packages/metadata-protocol/src/protocol-publish-drafts-package-scope.test.ts +++ b/packages/metadata-protocol/src/protocol-publish-drafts-package-scope.test.ts @@ -303,3 +303,93 @@ describe('publishPackageDrafts — two packages holding drafts for one (type, na expect(labelOf(active[0])).toBe('SOLO'); }); }); + +/** + * [#10350] The PER-ITEM door's half of the same key. + * + * `POST /meta/:type/:name/publish?package=PKG_ID` (#10063) made + * `publishMetaItem` a package-naming caller too, so the narrowing the cases + * above pin for `publishPackageDrafts` now has a SECOND entry point. The + * runtime path already carried the value — `publishMetaItem` forwards its + * whole request object and the one transform in between + * (`canonicalizeMetaRequestType`) is a spread that drops no key — but nothing + * pinned it, and the DECLARED request type did not carry `packageId` at all. + * + * ⚠️ What these cases ARE, stated so nobody reads more into a green run than + * it holds: they are REGRESSION PINS on a path that is already correct, NOT a + * defect control. There is no pre-fix red to show at runtime, and + * manufacturing one would misrepresent the card. The defect was on the TYPE + * surface, and its control is the compiler — before the declared shape carried + * `packageId`, the literal in the first case below did not typecheck + * (`TS2353`, `'packageId' does not exist in type ...`), which is exactly why + * the only caller that states one is a REST door reaching it through a cast. + * + * What makes these pins able to FAIL is the fixture they inherit: with the + * package dimension absent from the promote's lookup the first-scanned row + * (`app.other`) wins, which is the wrong one. A pin that only asserted + * `success: true` would pass either way — and the failure mode this card names + * is precisely a future refactor that DESTRUCTURES the request instead of + * forwarding it wholesale, dropping the key while every existing assertion + * stays green. + */ +describe('publishMetaItem — the per-item door names a package too (#10350)', () => { + it('promotes the STATED package draft, not the first row that shares the name', async () => { + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + await seedTwoPackageDrafts(protocol); + + const res = await protocol.publishMetaItem({ + type: 'object', + name: 'shared_ticket', + packageId: 'app.demo', + }); + + expect(res).toMatchObject({ success: true }); + // Drop `packageId` on the way through `publishMetaItem` and the + // promote's lookup goes package-agnostic, landing on `app.other` — + // the same inversion the batch door carried before #8907. + const active = activeRowsOf(rows); + expect(active).toHaveLength(1); + expect(active[0].package_id).toBe('app.demo'); + expect(labelOf(active[0])).toBe('FROM_DEMO'); + }); + + it('drains the stated package own draft and leaves the other package draft pending', async () => { + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + await seedTwoPackageDrafts(protocol); + await protocol.publishMetaItem({ + type: 'object', + name: 'shared_ticket', + packageId: 'app.demo', + }); + + const drafts = draftRowsOf(rows); + expect(drafts).toHaveLength(1); + expect(drafts[0].package_id).toBe('app.other'); + expect(labelOf(drafts[0])).toBe('FROM_OTHER'); + }); + + it('keeps the historical match-any resolution when the caller states NO package', async () => { + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + await seedTwoPackageDrafts(protocol); + + await protocol.publishMetaItem({ type: 'object', name: 'shared_ticket' }); + + // The contract `promoteDraftForPublish` spells as + // `...('packageId' in request ? ... : {})`: an ABSENT key means "match + // any package" (this fixture's first-scanned row), while a + // present-and-`undefined` key would coerce to `null` downstream and pin + // the lookup to UNBOUND rows — finding neither draft and answering + // `no_draft`. Declaring `packageId` optional must not turn the first + // spelling into the second, so the untouched path is pinned here. + const active = activeRowsOf(rows); + expect(active).toHaveLength(1); + expect(active[0].package_id).toBe('app.other'); + expect(labelOf(active[0])).toBe('FROM_OTHER'); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 88495c3d19..9d4f22e8d9 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -4467,10 +4467,22 @@ export class ObjectStackProtocolImplementation implements requestOrgId: string | null, ): Promise { if (requestOrgId === null) return null; - // The package dimension is deliberately absent from both probes: the - // per-item door names no package, so `promoteDraft` resolves the draft - // with "match any package" and these reads must ask the same question - // it will (see `SysMetadataRepository.whereFor`). + // The package dimension is absent from both probes, and the rule + // behind that is the LIVE one: these reads must ask the same question + // `promoteDraft` will (see `SysMetadataRepository.whereFor`), because + // their whole job is to name the scope the promote then addresses. A + // probe NARROWER than the promote hides a draft the promote can see; a + // probe WIDER names a scope it cannot. + // + // [#10350] ⚠️ The justification this comment used to carry — "the + // per-item door names no package" — is STALE. Since #10063 + // `publishMetaItem` accepts a `packageId` and forwards it, so when the + // caller states one the promote IS package-scoped while these probes + // stay package-agnostic, and the two can then ask different questions. + // Behaviour is deliberately UNCHANGED here: closing that asymmetry is + // its own fix with its own fixture, filed separately rather than ridden + // in on a comment repair. What is corrected is the claim, so the next + // reader does not conclude the per-item door still cannot name one. const inOrg = await this.engine.findOne('sys_metadata', { where: { organization_id: requestOrgId, type: singularType, name, state: 'draft' }, }); @@ -14264,6 +14276,44 @@ export class ObjectStackProtocolImplementation implements organizationId?: string; actor?: string; message?: string; + /** + * [#10350] ADR-0048 — the software package the draft being promoted was + * listed under, when the caller has one to state. Forwarded whole to + * {@link promoteDraftForPublish}, which threads it into BOTH the #9612 + * gate closure and `repo.promoteDraft`, so the gate and the write + * resolve the draft under the SAME key it was listed by. + * + * Declared because it is REAL on this door, not merely tolerated: + * since #10063 `POST /meta/:type/:name/publish?package=PKG_ID` states it + * on every HTTP-driven promotion that names a package — which is + * Studio's designer save-then-publish loop. Until it was declared the + * value flowed correctly but was invisible to every typed caller, and + * the only caller that states one reaches this method through a cast, + * so the binding was enforced by nothing. `#10350` added the pins in + * `protocol-publish-drafts-package-scope.test.ts`. + * + * ⚠️ `null` is NOT the same as absent, and the difference is load + * bearing the whole way down: {@link promoteDraftForPublish} branches on + * the KEY BEING PRESENT (`'packageId' in request`), so an ABSENT key + * keeps the historical "match any package" resolution while `null` pins + * the lookup to the UNBOUND row. Spread it in conditionally; never write + * `packageId: maybeUndefined`, which arrives as a present-and-undefined + * key, coerces to `null` downstream, and makes a package-bound draft + * unfindable — a silent `no_draft` on the untouched path. + */ + packageId?: string | null; + // [#10350] `environmentId` is deliberately NOT declared here, although + // the REST door spreads it into this very request literal. It is the + // multi-kernel ROUTING key, and it is out of the protocol request shape + // by explicit maintainer ruling (recorded 2026-08-18 on #9741): + // `resolveProtocol(environmentId)` has already selected the target + // kernel before this method is entered, and this class reads its + // environment off the INSTANCE (`this.environmentId`, set at + // construction) and never off a request — measured, `request.environmentId` + // occurs nowhere in this file. `packages/rest` declares that one + // transport-level member on top of the declared shape instead + // (`TransportScopedMetaRequest`), which is where a routing key belongs. + // Adding it here would reverse that ruling rather than record it. /** * INTERNAL — `publishPackageDrafts` publishes many drafts and batch-applies * every seed body in ONE loader pass afterwards (cross-seed references need @@ -14545,10 +14595,14 @@ export class ObjectStackProtocolImplementation implements * SAME key it was listed by, exactly as `organizationId` above threads * the draft's own org scope for the #3115 partition analogue. * - * `undefined` (the `publishMetaItem` path, which names no package) - * keeps the historical "match any package" resolution. `null` pins the - * lookup to the unbound row — so the field is passed through only when - * the caller actually has a binding to state. + * `undefined` (any caller with no binding to state) keeps the + * historical "match any package" resolution. `null` pins the lookup to + * the unbound row — so the field is passed through only when the caller + * actually has a binding to state. [#10350] That parenthetical used to + * read "the `publishMetaItem` path, which names no package"; since + * #10063 the per-item door names one whenever its HTTP caller does, so + * `undefined` is now about the ABSENCE of a binding, never about which + * caller is on the other end. */ packageId?: string | null; /** @@ -14652,12 +14706,17 @@ export class ObjectStackProtocolImplementation implements // [#9612] The package binding the CALLER stated for this // promotion — the same value threaded into `repo.promoteDraft` // below, so the gate and the write resolve the draft under one - // key rather than two. `publishPackageDrafts` states it (a - // package publish, which is exactly the write this card is - // about); bare `publishMetaItem` names no package and so - // narrows nothing, which is the correct answer rather than a - // gap — a promotion whose package is unstated has no declared - // dependency set to bound it. + // key rather than two. BOTH callers can state it: + // `publishPackageDrafts` always does (a package publish, which + // is exactly the write #9612 was about), and [#10350] since + // #10063 `publishMetaItem` does too — whenever the HTTP caller + // named one on `POST /meta/:type/:name/publish?package=PKG_ID`. + // An UNSTATED package still narrows nothing, and that remains + // the correct answer rather than a gap — a promotion whose + // package is unstated has no declared dependency set to bound + // it. (This comment used to say the per-item door names no + // package at all, which stopped being true at #10063 and would + // read the REST door's forwarding as dead code.) // // ⚠️ Deliberately NOT read off `draftForGate`: `rowToItem` // projects `sys_metadata` into a `MetadataItem`, which carries From 4593fc4f3fd9e3b4a72ac6b5325b94dcec61b53b Mon Sep 17 00:00:00 2001 From: os-elon Date: Sat, 22 Aug 2026 03:46:37 +0000 Subject: [PATCH 2/3] docs(rest): record what the publishMetaItem cast is load-bearing for + changeset Measured: deleting the cast yields `TS2339: Property 'publishMetaItem' does not exist on type 'RestProtocol'`, not a TS2353 about an unknown key. It is ADR-0076 D9 feature detection for a server-only extension `packages/spec` declares no member for - unchanged by declaring `packageId` on the implementation's request type. Removing it is a `packages/spec` contract decision; #10350 is not addressed by this comment beyond recording the measurement. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM --- .../publish-meta-item-declares-package-id.md | 43 +++++++++++++++++++ packages/rest/src/rest-server.ts | 27 ++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 .changeset/publish-meta-item-declares-package-id.md diff --git a/.changeset/publish-meta-item-declares-package-id.md b/.changeset/publish-meta-item-declares-package-id.md new file mode 100644 index 0000000000..9de2a09d93 --- /dev/null +++ b/.changeset/publish-meta-item-declares-package-id.md @@ -0,0 +1,43 @@ +--- +"@objectstack/metadata-protocol": patch +"@objectstack/rest": patch +--- + +Declare `packageId` on `publishMetaItem`'s request type, and correct three +in-tree comments that claimed the per-item publish door "names no package" +(#10350). + +`publishMetaItem` declared `type / name / organizationId / actor / message / +_skipSeedApply` and no `packageId`, while since #10063 `POST +/meta/:type/:name/publish?package=PKG_ID` states one on every HTTP-driven +promotion that names a package — which is Studio's designer save-then-publish +loop. + +**No runtime behaviour changes, and nothing was broken at runtime.** The value +already flowed end to end: `publishMetaItem` forwards its whole request object, +the one transform in between (`canonicalizeMetaRequestType`) is a spread that +drops no key, and `promoteDraftForPublish` already declared +`packageId?: string | null` and threaded it into both the #9612 gate closure and +`repo.promoteDraft`. What was wrong was the *declared* contract: the binding was +invisible to every typed caller, and the only caller that states one reaches the +method through a cast, so it was enforced by nothing — one destructuring +refactor away from being dropped in silence. + +`packageId` is `string | null | undefined`, and the three states are distinct: +an **absent** key keeps the historical "match any package" resolution, `null` +pins the lookup to the unbound row, and a present-and-`undefined` key coerces to +`null` downstream and makes a package-bound draft unfindable. Spread it in +conditionally; never write `packageId: maybeUndefined`. + +`environmentId` is deliberately **not** added, though it sits in the same +cast-hidden position on the REST call site. It is the multi-kernel routing key +and is out of the protocol request shape by the maintainer ruling recorded +2026-08-18 on #9741 — `resolveProtocol(environmentId)` selects the kernel before +the call, and `request.environmentId` is read nowhere in +`@objectstack/metadata-protocol`. `packages/rest` types that one transport-level +member on top of the declared shape (`TransportScopedMetaRequest`) instead. + +Three pins land in `protocol-publish-drafts-package-scope.test.ts`, on the same +two-colliding-drafts fixture the #8907 batch-door cases use, so a promote that +loses the package dimension resolves the *wrong* row rather than merely +succeeding. diff --git a/packages/rest/src/rest-server.ts b/packages/rest/src/rest-server.ts index 7554c924d4..6383c77eaa 100644 --- a/packages/rest/src/rest-server.ts +++ b/packages/rest/src/rest-server.ts @@ -5922,6 +5922,33 @@ export class RestServer { // org-scope comment for the measurement. canonicalMetaUrlType(req.params.type), ctx?.tenantId, ); + // [#10350] The cast stays, and what it is load-bearing + // FOR is worth stating so this is not re-filed as a missing + // contract. It is NOT hiding the request shape: measured by + // deleting it and running `pnpm --filter @objectstack/rest + // typecheck`, the compiler answers + // `TS2339: Property 'publishMetaItem' does not exist on + // type 'RestProtocol'` + // — not a TS2353 about an unknown key. `publishMetaItem` is + // an ADR-0076 D9 SERVER-ONLY extension: `RestProtocol` is + // `DataProtocol & MetadataProtocol`, and `MetadataProtocol` + // (`packages/spec`) declares no such member — only + // `PublishMetaItemResponseSchema` (#7294) exists there, with + // no request schema and no interface entry. So the cast is + // feature detection, exactly like the `auditMetaItem` twin + // a few hundred lines up, and the same measurement holds + // AFTER #10350 declared `packageId` on the implementation's + // request type: that type lives in + // `@objectstack/metadata-protocol`, which `packages/rest` + // deliberately does not depend on. + // + // Removing it therefore needs `MetadataProtocol` to declare + // the member (plus a `PublishMetaItemRequest` to hang the + // #9741 `TransportScopedMetaRequest` typing off) — a + // `packages/spec` contract decision, promoting an undeclared + // optional extension into a declared one, which is the same + // call the 501 refusal above declines to pre-empt. Filed + // rather than taken here. const result = await (p as any).publishMetaItem({ type: req.params.type, name: req.params.name, From ca53f9aad5a4e989ef284380072c56f2fd362a9d Mon Sep 17 00:00:00 2001 From: os-elon Date: Sat, 22 Aug 2026 03:48:22 +0000 Subject: [PATCH 3/3] docs(metadata-protocol): point the scope-probe comment at the filed follow-up The package-dimension asymmetry the corrected comment describes is now tracked as #11003; behaviour here stays untouched. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019yDEhPBC3tcGkW9bkce1HM --- packages/metadata-protocol/src/protocol.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 9d4f22e8d9..471929c05a 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -4480,8 +4480,9 @@ export class ObjectStackProtocolImplementation implements // caller states one the promote IS package-scoped while these probes // stay package-agnostic, and the two can then ask different questions. // Behaviour is deliberately UNCHANGED here: closing that asymmetry is - // its own fix with its own fixture, filed separately rather than ridden - // in on a comment repair. What is corrected is the claim, so the next + // its own fix with its own fixture and its own precedence ruling + // (ADR-0005 overlay order vs the ADR-0048 package key), filed as #11003 + // rather than ridden in on a comment repair. What is corrected is the claim, so the next // reader does not conclude the per-item door still cannot name one. const inOrg = await this.engine.findOne('sys_metadata', { where: { organization_id: requestOrgId, type: singularType, name, state: 'draft' },