diff --git a/.changeset/6965-batch-publish-advisories.md b/.changeset/6965-batch-publish-advisories.md new file mode 100644 index 0000000000..14e433cdff --- /dev/null +++ b/.changeset/6965-batch-publish-advisories.md @@ -0,0 +1,34 @@ +--- +'@object-ui/data-objectstack': minor +'@object-ui/app-shell': minor +--- + +Studio's "publish whole app" reports the runtime authoring gate's per-draft +advisories (objectui#6965; server half objectstack#9343). + +`POST /packages/:id/publish-drafts` began answering `advisories` on each +`published[]` element when objectstack#9343 landed, but the author publishing a +whole app was still told nothing: both client call sites bypassed the data-layer +seam — a bare `fetch` in `usePublishAllDrafts` and the page-private `apiJson` in +`PackagesPage`, whose declared response type held two counts and `failed[]`, with +no `published[]` at all. The same button's own client-side capability lint was +raising a toast the whole time, so a finding from the server was the one thing +that could not reach the person pressing it. + +- `MetadataClient.publishPackageDrafts(packageId)` expresses the route and emits + one advisory event per advised `published[]` element — each naming that + element's own `type` / `name` — into the sink, event and renderer the save and + single-item publish doors already use. Both call sites go through it. +- The batch door reports `door: 'publish'` rather than a third discriminator + value: every item the event names really was published, and the renderer's + only door-dependent output is that verb. The per-item identity the author + needs rides `type` / `name`, one event per item. +- It renders only what the server sent where `PublishPackageDraftsResponseSchema` + declares it. A half-shaped finding, an element that cannot name its item, and a + top-level `advisories` the ruled shape does not put there all report nothing — + pinned, alongside the presence, in `metadata-client.publishAdvisories.test.ts`, + whose absence pin this flips. +- `publishPackageDrafts` returns the batch body derived from the spec schema, so + a caller reading `failed[]` / `publishedCount` reads a declared shape. Non-2xx + raises the usual `MetadataError`; the 2xx batch verdict stays the caller's to + judge, because `success: false` is not a refusal on this route. diff --git a/.changeset/render-publish-advisory-findings-5026.md b/.changeset/render-publish-advisory-findings-5026.md index 659dd337e1..887dc04278 100644 --- a/.changeset/render-publish-advisory-findings-5026.md +++ b/.changeset/render-publish-advisory-findings-5026.md @@ -16,4 +16,6 @@ One thing had to differ, and it is the frame's verb. Save and Publish are two di **BREAKING for event constructors — `MetadataSaveAdvisoryEvent.door` is required.** Reading the event is unaffected: a listener that ignores `door` behaves exactly as before, and every other member is unchanged. Constructing one is a compile break — a door-less event literal that type-checked before now fails with TS2741, `Property 'door' is missing`. Measured on the emitted `dist/index.d.ts` of `@object-ui/data-objectstack` on both sides: that single required member is the entire non-comment delta of the package's published surface. **Migration:** add `door: 'save'` or `door: 'publish'` to the literal, whichever write it models — `'save'` for `PUT /meta/:type/:name`, `'publish'` for `POST /meta/:type/:name/publish`. Scored `minor` rather than `major` per the repo's version policy: objectui's major is pinned to `@objectstack`'s so that "same major means compatible" holds across the two repos, so objectui's own breaking changes ship as `minor` with the break named here (`scripts/check-changeset-no-major.mjs`). Every publishable package sits in one `fixed` group, so this entry carries the group. -Unchanged, deliberately: the **batch** door. "Publish whole app" (`POST /packages/:id/publish-drafts`) still discards per-draft advisories server-side — objectstack#9343, open and unruled — and nothing here compensates for that from the client side. A test pins the absence, so a later traversal of a batch-shaped `published[]` cannot be added without turning it red. +Unchanged, deliberately: the **batch** door. "Publish whole app" (`POST /packages/:id/publish-drafts`) discarded per-draft advisories server-side when this change was written — objectstack#9343, open and unruled at the time — and nothing here compensated for that from the client side. A test pinned the absence, so a later traversal of a batch-shaped `published[]` could not be added without turning it red. + +*Corrected before release (objectui#6965): both present-tense claims in the paragraph above went false after it was written — objectstack#9343 landed, the batch response now carries per-draft advisories, and objectui#6965 routes that door through the same seam and flips the absence pin to a presence pin. The paragraph is kept in the past tense as the record of what this change did and did not do; this note is what the CHANGELOG publishes instead of a sentence that was true only while it sat here.* diff --git a/packages/app-shell/src/preview/usePublishAllDrafts.ts b/packages/app-shell/src/preview/usePublishAllDrafts.ts index bf68f36d52..282d4fe4c3 100644 --- a/packages/app-shell/src/preview/usePublishAllDrafts.ts +++ b/packages/app-shell/src/preview/usePublishAllDrafts.ts @@ -16,6 +16,13 @@ * L3 runtime probes; findings surface as a loud warning toast instead of a * blind "Published!". Package-less drafts fall back to by-reference publish * (structure first, seeds last) so they never dead-end. + * + * Both halves of that call now run through `MetadataClient` (objectui#6965): + * the batch one so the runtime authoring gate's per-draft advisories reach the + * console's advisory toast, the by-reference one because it always did. The + * asymmetry this closes was inside this very function — its own client-side + * capability lint raised a toast while the server's findings, on the same + * button, were dropped for want of a seam to report through. */ import { useCallback, useState } from 'react'; @@ -74,15 +81,20 @@ export function usePublishAllDrafts(t: TranslateFn) { }; for (const packageId of packageIds) { - const res = await fetch(`/api/v1/packages/${encodeURIComponent(packageId)}/publish-drafts`, { - method: 'POST', - credentials: 'include', - headers: { 'Content-Type': 'application/json', Accept: 'application/json' }, - body: '{}', - }); - const payload = await res.json().catch(() => null); - if (!res.ok || (payload as any)?.success === false) { - throw new Error((payload as any)?.error?.message || `HTTP ${res.status}`); + // objectui#6965 — through `MetadataClient`, not a bare `fetch`. The + // route now answers the runtime authoring gate's per-draft advisories + // on each `published[]` element (objectstack#9343), and the client is + // the seam that reports them: it emits one advisory event per advised + // item into the same sink, renderer and wording the save and + // single-item publish doors use. A bare fetch had nothing to report + // THROUGH — which is why this door stayed silent while the L3 probe + // findings a few lines below were already shouting. + const payload = await client.publishPackageDrafts(packageId); + // A non-2xx now throws inside the client, with the server's own + // message. What is left to check here is the batch verdict, unchanged. + if ((payload as { success?: boolean }).success === false) { + const error = (payload as { error?: { message?: string } }).error; + throw new Error(error?.message || 'publish-drafts did not publish this package'); } recordHealth(publishHealthFromResponse(payload)); } diff --git a/packages/app-shell/src/views/metadata-admin/PackagesPage.tsx b/packages/app-shell/src/views/metadata-admin/PackagesPage.tsx index 8205733132..a1a12c5ca2 100644 --- a/packages/app-shell/src/views/metadata-admin/PackagesPage.tsx +++ b/packages/app-shell/src/views/metadata-admin/PackagesPage.tsx @@ -61,6 +61,7 @@ import { SheetDescription, } from '@object-ui/components'; import { useMetadataLocale, t, tFormat } from './i18n.js'; +import { useMetadataClient } from './useMetadata.js'; import { PackageFormDialog } from './PackageFormDialog.js'; import { errorCodeIs } from '@object-ui/types'; import { readEnvelopeFailureText } from '../../utils/apiErrorEnvelope.js'; @@ -280,6 +281,14 @@ export function PackageDetailSheet({ onChanged: () => void; }) { const locale = useMetadataLocale(); + // objectui#6965 — the console's metadata client, for the ONE action on this + // sheet that must report: "publish drafts" promotes metadata, and the runtime + // authoring gate's findings for those promotions ride the response. This hook + // is where the advisory sink is wired (`useMetadataClient` → the toast + // renderer), so a call made through it reports and a call made through the + // page-private `apiJson` cannot. The other lifecycle actions on this sheet + // write no metadata and stay on `apiJson`. + const client = useMetadataClient(); const [busy, setBusy] = React.useState(null); const [msg, setMsg] = React.useState<{ kind: 'ok' | 'err'; text: string } | null>(null); // ADR-0033 — pending DRAFT items bound to this package. AI-authored metadata @@ -360,47 +369,68 @@ export function PackageDetailSheet({ // ADR-0033 — publish every pending draft of this app in one shot, then // refresh the pending list (it should now be empty). Distinct from the // registry-based `publish` above; this hits `/publish-drafts`. + // + // objectui#6965 — through `MetadataClient`, not `apiJson`. This promotes + // metadata, so the runtime authoring gate grades it and answers its findings + // on each `published[]` element (objectstack#9343); the client is the seam + // that reports them to the author. `apiJson` could not — and the response + // type declared here could not even hold them: it listed the two counts and + // `failed[]`, with no `published[]` at all. The declared shape now comes from + // the spec, through the client's return type. const publishDrafts = () => run( 'publish-drafts', - () => - apiJson<{ - publishedCount?: number; - failedCount?: number; - failed?: Array<{ type?: string; name?: string; error?: string; code?: string }>; - }>( - `${API}/${encodeURIComponent(id)}/publish-drafts`, - { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({}) }, - ).then(async (r) => { - try { - const fresh = await apiJson<{ drafts?: Array<{ type: string; name: string }> }>( - `/api/v1/meta/_drafts?packageId=${encodeURIComponent(id)}`, - ); - setDrafts(fresh?.drafts ?? []); - } catch { - setDrafts([]); - } - if (r?.failedCount) { - // framework 15.1+ (ADR-0067 D2): the batch is all-or-nothing — a - // failure means NOTHING landed and `failed[]` marks the rolled-back - // drafts `batch_aborted`, with the causal item carrying the real - // error. Say "rolled back because X", not "{n} failed" (which reads - // as a partial publish that no longer exists). - const failedList = Array.isArray(r.failed) ? r.failed : []; - const causal = failedList.find((f) => !errorCodeIs(f, 'BATCH_ABORTED') && f?.error); - if (failedList.some((f) => errorCodeIs(f, 'BATCH_ABORTED'))) { - throw new Error(tFormat('engine.packages.detail.publishDraftsRolledBack', locale, { - cause: causal ? `${causal.type ?? '?'}/${causal.name ?? '?'}: ${causal.error}` : String(r.failedCount), - })); - } - // pre-15.1 server — genuine partial publish. - throw new Error(tFormat('engine.packages.detail.publishDraftsPartial', locale, { - published: r.publishedCount ?? 0, - failed: r.failedCount, + async () => { + const r = await client.publishPackageDrafts(id).catch((e: unknown) => { + // The ADR-0112 rule objectui#7959 landed on this page: a + // producer-marked `error.userMessage` outranks the diagnostic + // `error.message`. `MetadataClient` raises with the diagnostic and + // keeps the body, so the marked sentence is re-read here rather + // than lost on the way through the seam. + const marked = readEnvelopeFailureText((e as { body?: unknown } | null)?.body); + throw marked ? new Error(marked) : e; + }); + if ((r as { success?: boolean }).success === false) { + // Preserves what `apiJson` did for this call: a batch that did not + // publish is an error on this surface, read through the same + // envelope ladder. The status is no longer in hand — a non-2xx + // threw above — so the last rung is a sentence, not "(200)". + throw new Error( + readEnvelopeFailureText(r) || + (typeof r.error === 'string' ? r.error : '') || + (typeof r.message === 'string' ? r.message : '') || + t('engine.packages.detail.actionFailed', locale), + ); + } + try { + const fresh = await apiJson<{ drafts?: Array<{ type: string; name: string }> }>( + `/api/v1/meta/_drafts?packageId=${encodeURIComponent(id)}`, + ); + setDrafts(fresh?.drafts ?? []); + } catch { + setDrafts([]); + } + if (r?.failedCount) { + // framework 15.1+ (ADR-0067 D2): the batch is all-or-nothing — a + // failure means NOTHING landed and `failed[]` marks the rolled-back + // drafts `batch_aborted`, with the causal item carrying the real + // error. Say "rolled back because X", not "{n} failed" (which reads + // as a partial publish that no longer exists). + const failedList = Array.isArray(r.failed) ? r.failed : []; + const causal = failedList.find((f) => !errorCodeIs(f, 'BATCH_ABORTED') && f?.error); + if (failedList.some((f) => errorCodeIs(f, 'BATCH_ABORTED'))) { + throw new Error(tFormat('engine.packages.detail.publishDraftsRolledBack', locale, { + cause: causal ? `${causal.type ?? '?'}/${causal.name ?? '?'}: ${causal.error}` : String(r.failedCount), })); } - return r; - }), + // pre-15.1 server — genuine partial publish. + throw new Error(tFormat('engine.packages.detail.publishDraftsPartial', locale, { + published: r.publishedCount ?? 0, + failed: r.failedCount, + })); + } + return r; + }, t('engine.packages.detail.publishDraftsOk', locale), ); diff --git a/packages/data-objectstack/src/metadata-client.publishAdvisories.test.ts b/packages/data-objectstack/src/metadata-client.publishAdvisories.test.ts index 8d033b90c0..2b24f5a8cd 100644 --- a/packages/data-objectstack/src/metadata-client.publishAdvisories.test.ts +++ b/packages/data-objectstack/src/metadata-client.publishAdvisories.test.ts @@ -29,17 +29,36 @@ * from `publish()` / `publishDraft()`, every "emits" case below fails because no * event ever arrives. * - * ## Scope control — the batch door is NOT this + * ## The BATCH door reports too, and this file pins both halves (objectui#6965) * - * `POST /packages/:id/publish-drafts` ("publish whole app") still discards - * per-draft advisories SERVER-side; that is objectstack#9343, open and - * unruled at the time of writing, and nothing on this side compensates for it. - * The last case in this file is the control that pins that absence: a - * batch-shaped body reaching this client renders nothing. + * This section used to say that `POST /packages/:id/publish-drafts` ("publish + * whole app") still discarded per-draft advisories SERVER-side, and the last + * case in this file pinned that absence: a batch-shaped body reaching this + * client rendered nothing. That sentence was the absence pin's whole reason, + * and objectstack#9343 falsified it — the batch response now carries + * `advisories` on EACH `published[]` element, declared by + * `PublishPackageDraftsResponseSchema` in the INSTALLED `@objectstack/spec`. + * + * So the absence is flipped to a presence, at the door that owns the route: + * {@link MetadataClient.publishPackageDrafts} emits one event per advised + * element. What the flip must NOT lose is what the absence was really + * protecting — that the client renders only what the server sent, where the + * server's own schema declares it. Both halves are pinned below: + * + * - The batch door renders findings that arrived on a body the spec accepts, + * and renders NOTHING it had to invent — a half-shaped finding, an element + * that cannot name its item, a top-level `advisories` the ruled shape does + * not put there. + * - The single-item door still does not dig into `published[]`. Its own + * response schema declares no such key, and "look wherever a finding might + * be" is the contract-inventing move the original pin was built to block. */ import { describe, it, expect, vi } from 'vitest'; -import { PublishMetaItemResponseSchema } from '@objectstack/spec/api'; +import { + PublishMetaItemResponseSchema, + PublishPackageDraftsResponseSchema, +} from '@objectstack/spec/api'; import { MetadataClient, type MetadataSaveAdvisoryEvent, @@ -56,6 +75,16 @@ const PURGE_ADVISORY: RuntimeAuthoringIssue = { hint: 'add a filter, or set multi: false to delete a single record', }; +/** A second finding, so a per-element assertion cannot pass by coincidence. */ +const CASES_ADVISORY: RuntimeAuthoringIssue = { + severity: 'warning', + rule: 'view/column-references-missing-field', + where: 'view "cases" · column 3', + path: 'columns[2].field', + message: 'this column binds `owner_name`, which the object does not declare', + hint: 'bind an existing field, or add `owner_name` to the object', +}; + /** The three keys `PublishMetaItemResponseSchema` states as REQUIRED. */ const CLEAN_BODY = { success: true, @@ -64,6 +93,35 @@ const CLEAN_BODY = { message: 'Published draft — type=flow, name=nightly_purge [seq=7]', }; +/** An ADR-0008 content hash, in the format the batch door returns per element. */ +const VERSION = 'sha256:1a2b3c4d5e6f70819293a4b5c6d7e8f91a2b3c4d5e6f70819293a4b5c6d7e8f9'; + +/** + * A "publish whole app" body with findings on ONE of two promoted elements — + * the six keys `PublishPackageDraftsResponseSchema` states as REQUIRED, plus + * the optional `advisories` where the ruling puts them. + * + * Built as a function so a case can vary one element without the others + * drifting, and asserted against the installed schema below rather than + * trusted: a fixture nothing validates is how a client ends up pinning its own + * imagination. + */ +function batchBody( + published: Array> = [ + { type: 'view', name: 'cases', version: VERSION }, + { type: 'flow', name: 'nightly_purge', version: VERSION, advisories: [PURGE_ADVISORY] }, + ], +) { + return { + success: true, + outcome: 'published', + publishedCount: published.length, + failedCount: 0, + published, + failed: [], + }; +} + function response(body: unknown): Response { return new Response(JSON.stringify(body), { status: 200, @@ -123,6 +181,56 @@ describe('the contract this renders (objectstack#9176), read off the installed s }); }); +/** + * The batch door's premise, asserted against the INSTALLED spec for the same + * reason its single-item sibling above is: this half of objectui#6965 was held + * on objectstack#9343 landing, and what releases it is not that card's state + * but the shape a consumer can actually install and read. + * + * ⭐ The third case is the one that keeps the client honest. The ruling was + * explicit that advisories ride EACH element and that there is NO parallel + * top-level map; a schema that merely tolerated a top-level key would make + * "read the elements" a style preference instead of the contract. + */ +describe('the batch contract this renders (objectstack#9343), read off the installed spec', () => { + it('declares `advisories` on each `published[]` element, and validates its elements', () => { + const parsed = PublishPackageDraftsResponseSchema.safeParse(batchBody()); + expect(parsed.success).toBe(true); + const advised = parsed.success + ? (parsed.data.published[1] as Record) + : undefined; + expect(advised && Object.prototype.hasOwnProperty.call(advised, 'advisories')).toBe(true); + + // The reverse probe: without it, "the key parses" would prove only that + // the schema ignores what is under it. + const halfShaped = PublishPackageDraftsResponseSchema.safeParse( + batchBody([ + { type: 'flow', name: 'nightly_purge', version: VERSION, advisories: [{ rule: 'only-a-rule' }] }, + ]), + ); + expect(halfShaped.success).toBe(false); + }); + + it('omits the key on a clean element — absence means "nothing to report"', () => { + const parsed = PublishPackageDraftsResponseSchema.safeParse(batchBody()); + const clean = parsed.success ? (parsed.data.published[0] as Record) : undefined; + expect(clean && Object.prototype.hasOwnProperty.call(clean, 'advisories')).toBe(false); + }); + + it('declares NO parallel top-level `advisories` — the ruled shape, not a preference', () => { + const parsed = PublishPackageDraftsResponseSchema.safeParse({ + ...batchBody(), + advisories: [PURGE_ADVISORY], + }); + // Undeclared keys are stripped, so a surviving key would mean the schema + // declares one. It does not — which is why the client reads the elements. + expect(parsed.success).toBe(true); + expect(parsed.success && Object.prototype.hasOwnProperty.call(parsed.data, 'advisories')).toBe( + false, + ); + }); +}); + describe('MetadataClient.publish — runtime authoring gate advisories (#5026)', () => { it('emits the findings a successful promotion returned', async () => { const events: MetadataSaveAdvisoryEvent[] = []; @@ -311,33 +419,219 @@ describe('MetadataClient.publishDraft — the same door, so the same report', () }); /** - * The scope control, and it is a real one rather than a restatement. + * The door control — what the flipped pin below must NOT take with it. * - * "Publish whole app" is `POST /packages/:id/publish-drafts`, a route this - * client class does not express at all — `usePublishAllDrafts` calls it with - * a bare `fetch`. Its response reports per-draft results under `published[]`, - * and those elements carry no advisories server-side (objectstack#9343). + * "Publish whole app" is `POST /packages/:id/publish-drafts`, and since + * objectui#6965 this client expresses it: {@link + * MetadataClient.publishPackageDrafts}, pinned in the next describe. That + * says nothing about THIS method, which answers a different route whose + * response schema declares no `published[]` at all. * - * If a batch-shaped body ever reached this method, nothing here may go - * hunting through `published[]` for findings to render: that would be the - * batch rendering this card explicitly excluded, built on a side-channel - * instead of on a contract. Pinned as an absence so a later "helpful" - * traversal cannot be added without turning this red. + * So if a batch-shaped body ever arrives here, nothing may go hunting + * through it for findings to render. Reading the place one's own contract + * declares is what separates rendering from inventing, and a traversal added + * "helpfully" to the single-item door turns this red. */ - it('does NOT render advisories buried in a batch-shaped `published[]` body', async () => { + it('does NOT dig into a batch-shaped `published[]` body — wrong door, undeclared key', async () => { + const events: MetadataSaveAdvisoryEvent[] = []; + const client = clientWith(batchBody(), (e) => events.push(e)); + + await client.publishDraft('flow', 'nightly_purge'); + + expect(events).toEqual([]); + }); +}); + +/** + * THE FLIPPED PIN (objectui#6965) — same fixture, same question, inverted + * answer, now asked of the door that owns the route. + * + * It was an ABSENCE pin: "a batch-shaped body reaching this client renders + * nothing", and its stated reason was that `POST /packages/:id/publish-drafts` + * discarded per-draft advisories server-side. objectstack#9343 landed and + * retired that reason. Deleting the pin would have dropped the guarantee it + * was carrying alongside the absence — that the client renders only findings + * the server actually sent — so it is flipped rather than removed, and the + * cases below assert BOTH directions: + * + * - present, when the server sent them where the spec declares them; + * - absent, for everything the client would have had to invent. + * + * ⭐ Red-then-green, because one green proves nothing about a pin that was + * already passing: the old assertion (`expect(events).toEqual([])`) was run + * against this new door first and FAILS — the flip is a real behaviour change, + * not a rewording. That reading is quoted in the pull request. + */ +describe('MetadataClient.publishPackageDrafts — the BATCH door reports (objectui#6965)', () => { + it('renders the advisories the server sent on a `published[]` element', async () => { + const body = batchBody(); + // The fixture is the contract, not a guess: it parses against the spec the + // consumer has installed, so this pin cannot outlive the shape it claims. + expect(PublishPackageDraftsResponseSchema.safeParse(body).success).toBe(true); + const events: MetadataSaveAdvisoryEvent[] = []; + const client = clientWith(body, (e) => events.push(e)); + + await client.publishPackageDrafts('crm'); + + expect(events).toHaveLength(1); + expect(events[0]!.advisories).toEqual([PURGE_ADVISORY]); + }); + + it('names the item each finding is about — one event per advised element', async () => { const events: MetadataSaveAdvisoryEvent[] = []; const client = clientWith( - { - success: true, - publishedCount: 1, - failedCount: 0, - published: [{ type: 'flow', name: 'nightly_purge', advisories: [PURGE_ADVISORY] }], - }, + batchBody([ + { type: 'view', name: 'cases', version: VERSION, advisories: [CASES_ADVISORY] }, + { type: 'object', name: 'account', version: VERSION }, + { type: 'flow', name: 'nightly_purge', version: VERSION, advisories: [PURGE_ADVISORY] }, + ]), (e) => events.push(e), ); - await client.publishDraft('flow', 'nightly_purge'); + await client.publishPackageDrafts('crm'); + + // The author has to go fix a specific item, so the identity travels with + // the finding — and the clean element in the middle emits nothing. + expect(events.map((e) => `${e.type}/${e.name}`)).toEqual(['view/cases', 'flow/nightly_purge']); + expect(events[0]!.advisories).toEqual([CASES_ADVISORY]); + expect(events[1]!.advisories).toEqual([PURGE_ADVISORY]); + }); + + it('reports the PUBLISH door, so the frame reads "Published" and not "Saved"', async () => { + const events: MetadataSaveAdvisoryEvent[] = []; + const client = clientWith(batchBody(), (e) => events.push(e)); + + await client.publishPackageDrafts('crm'); + + // Reused rather than extended to a third value: every item this event + // names really was published, and the renderer's only door-dependent + // output is that verb. + expect(events[0]!.door).toBe('publish'); + expect(events[0]!.mode).toBe('publish'); + }); + + it('reads the elements through the dispatcher envelope this route declares', async () => { + const events: MetadataSaveAdvisoryEvent[] = []; + const client = clientWith({ success: true, data: batchBody() }, (e) => events.push(e)); + + const result = await client.publishPackageDrafts('crm'); + + // `PublishPackageDraftsResponseSchema` describes the body "inside the + // dispatcher's `{ success, data }` envelope" — so the declared object is + // the inner one, and both the findings and the returned value come from + // there. The single-item door's refusal to unwrap is the same rule read on + // its own route, not a disagreement. + expect(events).toHaveLength(1); + expect(result.publishedCount).toBe(2); + }); + it('says nothing about a batch whose elements carry no findings', async () => { + const events: MetadataSaveAdvisoryEvent[] = []; + const client = clientWith( + batchBody([{ type: 'view', name: 'cases', version: VERSION }]), + (e) => events.push(e), + ); + + const result = await client.publishPackageDrafts('crm'); + + // The zero is a reading only beside a control that must hit: the call went + // through and answered, so the silence is about the absent key. + expect(result.outcome).toBe('published'); expect(events).toEqual([]); }); + + it('INVENTS NOTHING: a half-shaped finding on an element is dropped, not rendered', async () => { + const events: MetadataSaveAdvisoryEvent[] = []; + const client = clientWith( + batchBody([ + { + type: 'flow', + name: 'nightly_purge', + version: VERSION, + advisories: [PURGE_ADVISORY, { rule: 'only-a-rule' }, null], + }, + ]), + (e) => events.push(e), + ); + + await client.publishPackageDrafts('crm'); + + // Half a finding would print blanks at the author, and completing one from + // the client side would be this client inventing server prose. + expect(events[0]!.advisories).toEqual([PURGE_ADVISORY]); + }); + + it('INVENTS NOTHING: an element that cannot name its item reports nothing', async () => { + const events: MetadataSaveAdvisoryEvent[] = []; + const client = clientWith( + batchBody([{ version: VERSION, advisories: [PURGE_ADVISORY] }]), + (e) => events.push(e), + ); + + await client.publishPackageDrafts('crm'); + + // `type` and `name` are REQUIRED on the element. An event has to name the + // item the author must go fix; one that cannot is worse than silence, and + // filling the gap with the package id or a placeholder would be a name the + // server never sent. + expect(events).toEqual([]); + }); + + it('INVENTS NOTHING: a top-level `advisories` is not the ruled shape and is not read', async () => { + const events: MetadataSaveAdvisoryEvent[] = []; + const client = clientWith( + { ...batchBody([{ type: 'view', name: 'cases', version: VERSION }]), advisories: [PURGE_ADVISORY] }, + (e) => events.push(e), + ); + + await client.publishPackageDrafts('crm'); + + // The ruling was "riding each element rather than a parallel top-level + // map", and the schema declares no such key (pinned above). Reading one + // anyway would render a finding out of a shape the server does not emit. + expect(events).toEqual([]); + }); + + it('a throwing sink never fails a batch the server already committed', async () => { + const client = clientWith(batchBody(), () => { + throw new Error('renderer exploded'); + }); + + await expect(client.publishPackageDrafts('crm')).resolves.toBeTruthy(); + }); + + it('refuses an empty packageId instead of firing a malformed request', async () => { + const fetchSpy = vi.fn(async (_url: string, _init?: RequestInit) => response(batchBody())); + const client = new MetadataClient({ + baseUrl: 'http://test.local', + fetch: fetchSpy as unknown as typeof fetch, + }); + + await expect(client.publishPackageDrafts('')).rejects.toThrow(/packageId must be non-empty/); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it('posts to the package route, unscoped by environment, like the call sites it replaces', async () => { + // Typed like `fetch` so `fetchSpy.mock.calls[0]` is `[url, init]` rather + // than an empty tuple (the zero-arg impl would otherwise infer `[]`, and + // indexing it is a compile error) — the spelling `exportDownload.test.ts` + // already uses, for the same reason it states there. `_url: string` rather + // than `RequestInfo | URL` because this client builds its URL as a string + // and the assertion below is meant to keep checking that. + const fetchSpy = vi.fn(async (_url: string, _init?: RequestInit) => response(batchBody())); + const client = new MetadataClient({ + baseUrl: 'http://test.local', + environmentId: 'env_1', + fetch: fetchSpy as unknown as typeof fetch, + }); + + await client.publishPackageDrafts('app.k9qk'); + + // The environment segment this client puts on `/meta` is deliberately NOT + // carried here: an `/environments/:id/packages` mirror is a route nothing + // in this repo has shown exists, and scoping to it would trade a working + // call for a 404. + const [url] = fetchSpy.mock.calls[0]!; + expect(url).toBe('http://test.local/api/v1/packages/app.k9qk/publish-drafts'); + }); }); diff --git a/packages/data-objectstack/src/metadata-client.ts b/packages/data-objectstack/src/metadata-client.ts index 3972243cf2..1d5fa99c3b 100644 --- a/packages/data-objectstack/src/metadata-client.ts +++ b/packages/data-objectstack/src/metadata-client.ts @@ -41,6 +41,7 @@ import { GetMetaItemLayeredResponseSchema } from '@objectstack/spec/api'; import { assertObjectMetadataWritable } from './object-metadata-write-guard'; import type { GetMetaItemLayeredResponse, + PublishPackageDraftsResponse, RuntimeAuthoringIssue, } from '@objectstack/spec/api'; @@ -85,6 +86,14 @@ export type { RuntimeAuthoringIssue }; * one event serve both. Measured against the installed `@objectstack/spec` * rather than assumed — see the PR for the probe. * + * ## And the BATCH publish door (objectui#6965) + * + * `PublishPackageDraftsResponseSchema` declares the same key with the same + * element type, riding EACH `published[]` element rather than a parallel + * top-level map (objectstack#9343's ruled shape). So the reader is still one + * reader — the object it is handed is one `published[]` element instead of a + * whole response — and the event is still one event per promoted item. + * * The name keeps its `Save` prefix because it is public API of this package and * renaming it would break consumers for no behavioural gain; {@link door} is * what says which write produced the event. @@ -107,8 +116,22 @@ export interface MetadataSaveAdvisoryEvent { * * - `'save'` — `PUT /meta/:type/:name` ({@link MetadataClient.save}, and the * SDK's `meta.saveItem` behind `ObjectStackAdapter`). - * - `'publish'` — `POST /meta/:type/:name/publish` - * ({@link MetadataClient.publish} and {@link MetadataClient.publishDraft}). + * - `'publish'` — the promotion doors: `POST /meta/:type/:name/publish` + * ({@link MetadataClient.publish} and {@link MetadataClient.publishDraft}), + * and the batch `POST /packages/:id/publish-drafts` + * ({@link MetadataClient.publishPackageDrafts}), which emits ONE event per + * `published[]` element carrying that element's own `type` / `name`. + * + * ## Why the batch route is not a third value (objectui#6965) + * + * The batch door promotes drafts to active — the author pressed Publish, and + * every item this event names really was published. What `door` decides is + * the frame's VERB, and "Published" is the true one for all three routes; a + * third value would have to render the same word. The per-item identity the + * author needs to act on is already carried by {@link type} / {@link name}, + * one event per item, so nothing about the batch is lost by sharing the + * value. ⛔ It is NOT a claim that "one call = one event": the batch emits as + * many events as it has advised items, which is why each one names its own. * * Distinct from {@link mode}, which cannot answer this: a direct active save * and a draft promotion both report `mode: 'publish'` because both land the @@ -133,10 +156,20 @@ export type MetadataSaveAdvisoryListener = (event: MetadataSaveAdvisoryEvent) => /** * Read the `advisories` array off a metadata write response, defensively. * - * Serves BOTH write doors unchanged (#5026): `SaveMetaItemResponseSchema` and - * `PublishMetaItemResponseSchema` declare the key at the same top level, under - * the same name, with the same element schema — so there is one reader, not a - * per-door copy that could drift. + * Serves EVERY write door unchanged (#5026, objectui#6965): + * `SaveMetaItemResponseSchema` and `PublishMetaItemResponseSchema` declare the + * key at the same top level, and `PublishPackageDraftsResponseSchema` declares + * it on each `published[]` element — under the same name, with the same element + * schema. So there is one reader, not a per-door copy that could drift; the + * batch caller hands it one element, which is the object that carries the key + * there. + * + * ⛔ It reads the `advisories` key of the object it is GIVEN and never + * traverses. Choosing the object — a response body at the single-item doors, + * one `published[]` element at the batch door — is the caller's job precisely + * because the place is declared per route: a reader that went hunting for + * findings wherever they might be would be inventing a contract the server + * never stated. * * The server omits the key entirely on a clean write, so `undefined` is the * common case and means "nothing to say". Anything that is not an array of @@ -188,10 +221,12 @@ export interface MetadataClientConfig { /** * Called after a {@link MetadataClient.save} whose 2xx response carried a * non-empty `advisories` array (objectstack#7435). Since #5026 the SAME sink - * also receives the publish door's findings (objectstack#9176) — read - * `event.door` to tell them apart. The write already - * succeeded; this is how the shell learns there is something to tell the - * author instead of the findings being discarded client-side. + * also receives the publish door's findings (objectstack#9176), and since + * objectui#6965 the batch publish door's too, one event per advised + * `published[]` element (objectstack#9343) — read `event.door` to tell a + * save from a promotion, and `event.type` / `event.name` for which item. The + * write already succeeded; this is how the shell learns there is something to + * tell the author instead of the findings being discarded client-side. * * Set on the CONFIG rather than exposed as a `subscribe()` method on purpose: * console metadata clients are minted per-component by `useMetadataClient`, @@ -223,6 +258,30 @@ export interface MetadataDraftHeader { updatedBy: string | null; } +/** + * What {@link MetadataClient.publishPackageDrafts} resolves with — the "publish + * whole app" body, DERIVED from `PublishPackageDraftsResponseSchema` rather + * than re-spelled (objectui#6965). + * + * Derived, and then widened in exactly two ways, each for a measured reason: + * + * - `Partial<…>` because this client talks to runtimes of several vintages and + * the spec's six required keys are what TODAY's producer sets. Declaring them + * required would be this client asserting a server version it cannot check — + * and an older runtime that omits one would be a type lie, not a compile + * error. Every consumer here already reads these keys defensively. + * - The index signature because the REST door adds keys on the way out + * (ADR-0045 visibility receipts, the `metadata:reloaded` announce receipt), + * and because a body this client hands back must stay readable by a caller + * that knows about a key this package has never heard of. + * + * ⛔ What it is NOT is a second definition of the batch response: the key names + * and element shapes come from the spec symbol, so a key the spec adds, renames + * or retypes arrives here with no edit. + */ +export type MetadataPublishPackageDraftsResult = Partial & + Record; + /** * Options for {@link MetadataClient.save} — a WRITE OVER HTTP to * `/api/v1/meta/:type/:name`. @@ -604,6 +663,54 @@ function buildBase(config: MetadataClientConfig): string { return `${trimmed}${scoped}`; } +/** + * The `/api/v1/packages` sibling of this client's `/meta` base (objectui#6965). + * + * Derived from the base rather than stored, so the two clone methods carry it + * for free. It keeps the ORIGIN the client was configured with (split-origin + * dev points the console at another port) and drops everything this client + * appended after it. + * + * ⚠️ The environment segment is dropped deliberately: the path built here is + * byte-for-byte the one the two batch-publish call sites in app-shell have + * always fired at, and an `/environments/:id/packages` mirror is a route this + * repo has no reading on. Scoping a call to a path nobody has shown exists + * would trade a working call for a 404. + */ +function packagesBaseOf(metaBase: string): string { + const origin = metaBase + .replace(/\/api\/v\d+(?:\/environments\/[^/]+)?\/meta$/, '') + .replace(/\/+$/, ''); + return `${origin}${API_PREFIX}/packages`; +} + +/** + * Unwrap the HTTP dispatcher's `{ success, data }` envelope — for the ONE route + * whose spec declaration says it arrives inside one (objectui#6965). + * + * ⛔ Not a general tolerance, and the difference is per-route rather than per + * file: {@link MetadataClient.publishDraft} refuses to unwrap because + * `PublishMetaItemResponseSchema` describes the full body of a route the REST + * server answers verbatim, so an envelope there is a shape that door does not + * serve. `PublishPackageDraftsResponseSchema` says the opposite in as many + * words — it "describes the FULL body … inside the dispatcher's + * `{ success, data }` envelope" — so the declared object is the INNER one, and + * unwrapping is how a caller gets the shape the spec declares. + * + * A body with no object-valued `data` is returned as-is: the REST composition + * answers this route unenveloped, and the ADR-0112 failure envelope + * (`{ success: false, error }`) has no `data` either, so its `error` stays + * where the caller's reader expects it. + */ +function unwrapDispatcherEnvelope(body: unknown): Record { + if (!body || typeof body !== 'object') return {}; + const root = body as Record; + const data = root.data; + return data && typeof data === 'object' && !Array.isArray(data) + ? (data as Record) + : root; +} + async function parseError(res: Response): Promise { let body: unknown; try { @@ -687,7 +794,7 @@ export class MetadataClient { private readonly headers: Record; /** ADR-0037: when true, reads render the draft-overlaid world. */ readonly previewDrafts: boolean; - /** #4133 / #5026 — sink for both write doors' advisory findings; see the config field. */ + /** #4133 / #5026 — sink for every write door's advisory findings; see the config field. */ private readonly onSaveAdvisory: MetadataSaveAdvisoryListener | undefined; constructor(config: MetadataClientConfig) { @@ -913,8 +1020,11 @@ export class MetadataClient { * calling method returns or whether it throws. The server emits `advisories` * ONLY when non-empty, so a clean write costs one absent-key check. * - * One helper rather than a copy per door: the two doors' responses declare - * the key identically, so a second inline copy could only ever drift. + * One helper rather than a copy per door: every door's response declares the + * key identically, so a second inline copy could only ever drift. The batch + * door (objectui#6965) calls it once per `published[]` element, handing it + * the element — the object that carries the key there — and the same + * best-effort contract covers that loop unchanged. */ private emitAdvisories( body: unknown, @@ -994,10 +1104,21 @@ export class MetadataClient { * rather than assume the data went live. * * Same door as {@link publish} (`POST /meta/:type/:name/publish`), so it - * reports the gate's advisories the same way (#5026). The BATCH door - * (`POST /packages/:id/publish-drafts`) is a different route that discards - * per-draft advisories server-side; that is objectstack#9343 and nothing here - * compensates for it. + * reports the gate's advisories the same way (#5026). + * + * The BATCH door (`POST /packages/:id/publish-drafts`) is a different route + * with a method of its own — {@link publishPackageDrafts} — and it reports + * too. It did not when this paragraph was first written: it discarded + * per-draft advisories server-side, which was objectstack#9343, and that card + * has since landed with a ruling that each `published[]` element carries + * them (objectui#6965 is the client half). + * + * What survives that change is the READING RULE, which is about this method + * rather than about the other route: it reads the top level of the + * single-item body and nothing else. A batch-shaped body arriving HERE still + * reports nothing, because `PublishMetaItemResponse` declares no + * `published[]` and a client that went looking for findings wherever they + * might be would be inventing a contract instead of reading one. * * ## Why there is no `{ success, data }` unwrapping here (objectui#6962) * @@ -1060,6 +1181,80 @@ export class MetadataClient { return body as any; } + /** + * Publish EVERY pending draft bound to one package — Studio's "publish whole + * app" door, `POST /api/v1/packages/:id/publish-drafts` (ADR-0033; the route + * that orders structure-before-seeds server-side and runs the ADR-0038 L3 + * runtime probes). + * + * ## Why it is expressed here at all (objectui#6965) + * + * It was not, and that was the defect. Two app-shell call sites fired this + * route with a bare `fetch` / a page-private `apiJson`, outside the seam that + * covers every other metadata write — so when objectstack#9343 landed and the + * server began sending per-draft advisories, the author publishing a whole + * app was told nothing, while the SAME button's client-side capability lint + * still raised a toast. A door that cannot report is not a door the gate can + * reach, however loudly the server speaks. + * + * ## What it reports, and what it refuses to invent + * + * `PublishPackageDraftsResponseSchema` declares `advisories` on EACH + * `published[]` element — objectstack#9343's ruled shape, the same element + * type and the same omitted-when-empty discipline as the single-item door, + * and explicitly not a parallel top-level map. So this method emits one + * {@link MetadataSaveAdvisoryEvent} per advised element, each naming that + * element's own `type` / `name`, through the same sink, event and renderer + * the save and single-item publish doors use. + * + * ⛔ It renders only what the server sent where the schema says it sits: + * + * - `advisories` is read off the element, by the one shared reader, which + * drops anything that is not a complete finding. + * - An element that does not carry the `type` and `name` the schema states as + * REQUIRED is skipped rather than reported under invented identity — the + * event names the item the author has to go fix, and an event that cannot + * name it truthfully is worse than silence. + * - Nothing is derived, counted or summarised from the batch: no advisory + * exists here that the server did not put in the body. + * + * ## The failure shapes stay the callers' + * + * Non-2xx throws {@link MetadataError}, like every other method. A 2xx is + * RETURNED unexamined — `success: false` is not a failure on this route + * (`outcome: 'nothing_to_publish'` answers it too, and the spec's own text + * says to read `outcome`, not the boolean), and the two callers have their + * own, different rules for the refusal and rolled-back cases. Deciding that + * here would change behaviour this card is not about. + */ + async publishPackageDrafts(packageId: string): Promise { + if (!packageId || !String(packageId).trim()) { + throw new Error( + 'MetadataClient.publishPackageDrafts: packageId must be non-empty.' + + ' The POST /packages/:id/publish-drafts route requires an id segment.', + ); + } + const url = `${packagesBaseOf(this.base)}/${encodeURIComponent(packageId)}/publish-drafts`; + const res = await this.fetchImpl(url, { + method: 'POST', + // Both call sites this replaces sent the session cookie, and the client's + // own fetch adds the console's Bearer token — so the routed call carries a + // superset of what the bare doors carried, never less. + credentials: 'include', + headers: { ...this.headers, 'Content-Type': 'application/json', Accept: 'application/json' }, + body: '{}', + }); + if (!res.ok) throw await parseError(res); + const body = unwrapDispatcherEnvelope(await res.json().catch(() => ({}))); + for (const element of Array.isArray(body.published) ? body.published : []) { + if (!element || typeof element !== 'object') continue; + const { type, name } = element as { type?: unknown; name?: unknown }; + if (typeof type !== 'string' || typeof name !== 'string') continue; + this.emitAdvisories(element, { type, name, door: 'publish', mode: 'publish' }); + } + return body as MetadataPublishPackageDraftsResult; + } + /** * Get the 3-state layered view of a metadata item: `code` (the packaged * artifact baseline), `overlay` (the tenant customisation row alone) and