Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions .changeset/publish-meta-item-declares-package-id.md
Original file line numberDiff line numberDiff line change
@@ -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.
Original file line numberDiff line numberDiff line change
Expand Up@@ -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');
});
});
88 changes: 74 additions & 14 deletions packages/metadata-protocol/src/protocol.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -4467,10 +4467,23 @@ export class ObjectStackProtocolImplementation implements
requestOrgId: string | null,
): Promise<string | null> {
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 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' },
});
Expand DownExpand Up@@ -14264,6 +14277,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
Expand DownExpand Up@@ -14545,10 +14596,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;
/**
Expand DownExpand Up@@ -14652,12 +14707,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
Expand Down
27 changes: 27 additions & 0 deletions packages/rest/src/rest-server.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -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,
Expand Down
Loading