diff --git a/.changeset/render-publish-advisory-findings-5026.md b/.changeset/render-publish-advisory-findings-5026.md new file mode 100644 index 0000000000..659dd337e1 --- /dev/null +++ b/.changeset/render-publish-advisory-findings-5026.md @@ -0,0 +1,19 @@ +--- +'@object-ui/data-objectstack': minor +'@object-ui/app-shell': patch +'@object-ui/i18n': patch +--- + +Studio surfaces the runtime authoring gate's advisory findings after a **publish**, not only after a save + +objectui#4133 / PR #4236 wired the gate's advisories to the save door and recorded, honestly, what that left unsurfaced: Studio's designer stages every edit as a `mode: 'draft'` save, drafts are never gated (the framework returns at its D1 early-return before a single rule runs), and the publish step that *is* gated returned no `advisories` field at all. So on the flow most tenants actually use, the author was told nothing at either door — for two different reasons, only one of which was objectui's. + +The second reason has expired. `PublishMetaItemResponseSchema` now declares the same optional, omitted-when-empty `advisories` key that `SaveMetaItemResponseSchema` has carried since #4717, and `publishMetaItem` populates it. Measured against the installed `@objectstack/spec` (17.2.0) rather than inferred from the version number: the key survives a `safeParse`, a half-shaped finding is rejected, and a clean publish omits the key entirely. That reading is now a test rather than a note, so a spec drift fails CI instead of silently re-muting the door. + +`MetadataClient.publish` and `MetadataClient.publishDraft` — the two methods over the single-item publish route `POST /meta/:type/:name/publish` — now report through the **same** sink, the same event and the same renderer the save door already used. No new UI shape: same warning tier, same 10s duration, same per-finding `rule` + `message` + `hint` formatting, findings still rendered verbatim as server prose. The wiring lands in the data layer rather than at the call sites, so `ResourceEditPage`'s Publish button and the runtime `RuntimeDraftBar` promotion (ObjectView / ReportView / DashboardView) are covered by one change, as are future ones. + +One thing had to differ, and it is the frame's verb. Save and Publish are two different buttons in this product, so a toast that says "Saved" after a Publish tells the author their change is still a draft — the opposite of what happened. `MetadataSaveAdvisoryEvent` therefore gains a required `door: 'save' | 'publish'` and the renderer picks `console.publishAdvisoryTitle` (added to all ten locale packs) accordingly. `door` exists because `mode` cannot answer this: a direct active save and a draft promotion both report `mode: 'publish'`, since both land the body in the active overlay. It is required rather than optional so a future third door cannot be wired without saying which one it is, and the renderer branches on it through an exhaustive switch with a `never` check, so adding a third member is a compile error rather than a silently wrong verb. + +**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. diff --git a/packages/app-shell/src/providers/saveAdvisoryToast.test.ts b/packages/app-shell/src/providers/saveAdvisoryToast.test.ts index a07c2c6369..155e719547 100644 --- a/packages/app-shell/src/providers/saveAdvisoryToast.test.ts +++ b/packages/app-shell/src/providers/saveAdvisoryToast.test.ts @@ -57,6 +57,7 @@ function event(overrides: Partial = {}): MetadataSave return { type: 'flow', name: 'nightly_purge', + door: 'save', mode: 'publish', advisories: [FINDING], ...overrides, @@ -160,3 +161,109 @@ describe('emitSaveAdvisories (#4133)', () => { expect(description).toContain(FINDING.message); }); }); + +/** + * The publish door renders through this SAME function (objectui#5026) — same + * warning tier, same duration, same per-finding formatting. One source more, + * not one surface more. What must differ is the frame's verb, because in this + * product Save and Publish are two different buttons: a toast that says "Saved" + * after a Publish tells the author their change is still a draft, which is the + * opposite of what happened. + */ +describe('emitSaveAdvisories — the publish door (#5026)', () => { + const published = () => event({ door: 'publish' }); + + it('says "Published", not "Saved", when the findings came through the publish door', () => { + const sink = makeSink(); + + emitSaveAdvisories(published(), t, sink); + + const [title] = sink.warning.mock.calls[0]!; + expect(title).toContain('Published'); + expect(title).not.toContain('Saved'); + }); + + it('keeps saying "Saved" for the save door — the existing wording is untouched', () => { + const sink = makeSink(); + + emitSaveAdvisories(event({ door: 'save' }), t, sink); + + expect(sink.warning.mock.calls[0]![0]).toContain('Saved'); + }); + + it('reads the DOOR, not the mode — both doors report `mode: "publish"`', () => { + // The discriminating case: a direct active save also carries + // `mode: 'publish'`, so a renderer that branched on `mode` would call it a + // publish. Same mode on both events here; only `door` differs. + const saveSink = makeSink(); + const publishSink = makeSink(); + + emitSaveAdvisories(event({ door: 'save', mode: 'publish' }), t, saveSink); + emitSaveAdvisories(event({ door: 'publish', mode: 'publish' }), t, publishSink); + + expect(saveSink.warning.mock.calls[0]![0]).toContain('Saved'); + expect(publishSink.warning.mock.calls[0]![0]).toContain('Published'); + }); + + it('is the same surface otherwise — warning tier, same body, same duration', () => { + const sink = makeSink(); + + emitSaveAdvisories(published(), t, sink); + + expect(sink.warning).toHaveBeenCalledTimes(1); + expect(sink.error).not.toHaveBeenCalled(); + const [, opts] = sink.warning.mock.calls[0]!; + expect(opts!.description).toContain(FINDING.message); + expect(opts!.description).toContain(FINDING.hint); + expect(opts!.duration).toBeGreaterThanOrEqual(10_000); + }); + + it('says nothing on a clean publish', () => { + const sink = makeSink(); + + emitSaveAdvisories(event({ door: 'publish', advisories: [] }), t, sink); + + expect(sink.warning).not.toHaveBeenCalled(); + }); +}); + +/** + * The exhaustiveness guarantee (#5026, contract-review condition 2). + * + * `door` being REQUIRED buys "every type-checked constructor must state a + * door". It does NOT by itself buy "the renderer handles the door it was + * given" — with a two-way ternary, a third union member would compile at its + * constructor, declare itself honestly, and still silently render "Saved", + * which is the exact class `door` exists to kill, reintroduced one level up. + * + * The compile-time half of the fix is the `never` check in `advisoryTitle`, + * enforced by `tsc` and not expressible here. What IS pinned here is its + * runtime consequence, which is what an untyped consumer would hit: an + * unhandled door must NOT come out wearing the save wording. + */ +describe('emitSaveAdvisories — the door union is handled exhaustively', () => { + it('refuses an unhandled door instead of silently calling it "Saved"', () => { + const sink = makeSink(); + // An untyped consumer's event. The cast is the point: inside the type + // system this is unreachable, which is what the `never` check enforces. + const rogue = event({ door: 'rollback' as unknown as MetadataSaveAdvisoryEvent['door'] }); + + expect(() => emitSaveAdvisories(rogue, t, sink)).toThrow(/advisory door/); + + // The load-bearing assertion: nothing was rendered. A wrong verb about a + // write that already touched the author's data is worse than no toast, + // and both emitters swallow this throw, so "no toast" is what ships. + expect(sink.warning).not.toHaveBeenCalled(); + expect(sink.error).not.toHaveBeenCalled(); + }); + + it('still handles every door the union actually declares', () => { + // The control for the case above: the refusal must be specific to an + // unhandled member, not a renderer that throws at everything. + for (const door of ['save', 'publish'] as const) { + const sink = makeSink(); + emitSaveAdvisories(event({ door }), t, sink); + expect(sink.warning).toHaveBeenCalledTimes(1); + } + }); +}); diff --git a/packages/app-shell/src/providers/saveAdvisoryToast.ts b/packages/app-shell/src/providers/saveAdvisoryToast.ts index 70c68d67ae..e32b714381 100644 --- a/packages/app-shell/src/providers/saveAdvisoryToast.ts +++ b/packages/app-shell/src/providers/saveAdvisoryToast.ts @@ -78,12 +78,66 @@ function formatFinding(f: MetadataSaveAdvisoryEvent['advisories'][number]): stri } /** - * Announce the gate's advisory findings for a save that SUCCEEDED. + * The frame's verb, chosen by the door the write came through. + * + * An exhaustive `switch` with a `never` check rather than a two-way ternary, + * and the difference is the whole point of the field. A ternary answers + * "is it publish, else save" — so a THIRD door added to the union would + * compile everywhere, declare itself honestly at its constructor, and still + * silently render "Saved". That is precisely the silent-wrong-verb class + * `door` exists to kill, reintroduced one level up. Here a new member makes + * this function a compile error instead, which is the only form of the + * guarantee worth having: the type must not merely be STATED, it must be + * HANDLED. + * + * The `default` branch is unreachable for type-checked callers — it exists + * for an untyped one (the event type is published, and JS consumers are not + * bound by it). It throws rather than falling back to the save wording, + * because both emitters wrap the sink in a try/catch that swallows: the + * failure mode is therefore "no toast", never "a toast that says the wrong + * thing about what just happened to the author's data". + */ +function advisoryTitle(ev: MetadataSaveAdvisoryEvent, t: TranslateFn): string { + const count = ev.advisories.length; + switch (ev.door) { + case 'save': + return t('console.saveAdvisoryTitle', { + count, + defaultValue: 'Saved — the authoring check raised {{count}} advisory finding(s)', + }); + case 'publish': + return t('console.publishAdvisoryTitle', { + count, + defaultValue: 'Published — the authoring check raised {{count}} advisory finding(s)', + }); + default: { + const unhandled: never = ev.door; + throw new Error( + `saveAdvisoryToast: no title for advisory door ${JSON.stringify(unhandled)}`, + ); + } + } +} + +/** + * Announce the gate's advisory findings for a metadata write that SUCCEEDED. * * Says nothing when there is nothing to say: the server omits `advisories` - * entirely on a clean save, so the common case never reaches here, and an event - * that somehow carried an empty list is dropped rather than toasted as + * entirely on a clean write, so the common case never reaches here, and an + * event that somehow carried an empty list is dropped rather than toasted as * "0 findings". + * + * ## One renderer, two doors (#5026) + * + * The publish door reports through this same function, the same warning tier, + * the same duration and the same per-finding formatting — a second SOURCE, not + * a second surface. Only the frame's verb changes, and it has to: Save and + * Publish are two different buttons in this product, so "Saved" after a Publish + * would tell the author their change is still a draft. `ev.door` is what says + * which one, because `ev.mode` cannot — a direct active save and a draft + * promotion both report `mode: 'publish'`. The choice is an exhaustive switch, + * not a two-way test: see {@link advisoryTitle} for why that distinction is + * the field's actual guarantee. */ export function emitSaveAdvisories( ev: MetadataSaveAdvisoryEvent, @@ -93,10 +147,7 @@ export function emitSaveAdvisories( if (!ev.advisories || ev.advisories.length === 0) return; sink.warning( - t('console.saveAdvisoryTitle', { - count: ev.advisories.length, - defaultValue: 'Saved — the authoring check raised {{count}} advisory finding(s)', - }), + advisoryTitle(ev, t), { description: ev.advisories.map(formatFinding).join('\n'), duration: ADVISORY_TOAST_MS, diff --git a/packages/data-objectstack/src/index.ts b/packages/data-objectstack/src/index.ts index 38f24fd4d3..c485403988 100644 --- a/packages/data-objectstack/src/index.ts +++ b/packages/data-objectstack/src/index.ts @@ -2750,6 +2750,12 @@ export class ObjectStackAdapter implements DataSource { this.emitSaveAdvisory({ type, name, + // #5026 — this interceptor wraps `meta.saveItem`, the SAVE door + // (`PUT /meta/:type/:name`) and only that one. The SDK's publish + // door (`meta.publishItem`) has no caller in this repo, so wiring + // it here would be a surface with no consumer; `MetadataClient` is + // where the publish door is actually taken. + door: 'save', mode: (result as { state?: string } | null | undefined)?.state === 'draft' ? 'draft' : 'publish', advisories, }); diff --git a/packages/data-objectstack/src/metadata-client.publishAdvisories.test.ts b/packages/data-objectstack/src/metadata-client.publishAdvisories.test.ts new file mode 100644 index 0000000000..aa36c2ff7b --- /dev/null +++ b/packages/data-objectstack/src/metadata-client.publishAdvisories.test.ts @@ -0,0 +1,327 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * The PUBLISH door must surface the runtime authoring gate's advisory findings + * (objectui#5026; contract objectstack#9176). + * + * ## Why this door is the one that matters + * + * objectui#4133 / PR #4236 wired the advisory rendering to the SAVE door and + * scoped this one out, because `PublishMetaItemResponseSchema` carried no + * `advisories` key at the time. It does now. That deferral left the common path + * silent, and for a precise reason both halves of which are pinned here: + * + * - Studio's designer stages every edit as a `mode: 'draft'` save, and drafts + * are NEVER gated — the framework returns at its D1 early-return before a + * rule runs, so the save door has nothing to report on that flow. + * - The promotion that follows IS gated. It is the write the gate actually + * grades, and until this change objectui parsed its response and dropped the + * findings on the floor exactly one layer further out than the server used to. + * + * So on the flow most tenants actually use, the author was told nothing at + * either door. These pins are the red-first evidence: with the emit removed + * from `publish()` / `publishDraft()`, every "emits" case below fails because no + * event ever arrives. + * + * ## Scope control — the batch door is NOT this + * + * `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. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { PublishMetaItemResponseSchema } from '@objectstack/spec/api'; +import { + MetadataClient, + type MetadataSaveAdvisoryEvent, + type RuntimeAuthoringIssue, +} from './metadata-client'; + +/** The measured `nightly_purge` finding, in the spec's D3 shape. */ +const PURGE_ADVISORY: RuntimeAuthoringIssue = { + severity: 'warning', + rule: 'flow/delete-without-filter', + where: 'flow "nightly_purge" · node "purge old rows"', + path: 'flows[0].nodes[2].config.filters', + message: 'this delete_record node sets multi: true with no filter, so it deletes every row', + hint: 'add a filter, or set multi: false to delete a single record', +}; + +/** The three keys `PublishMetaItemResponseSchema` states as REQUIRED. */ +const CLEAN_BODY = { + success: true, + version: 'sha256:0f1e2d3c4b5a69788796a5b4c3d2e1f00f1e2d3c4b5a69788796a5b4c3d2e1f0', + seq: 7, + message: 'Published draft — type=flow, name=nightly_purge [seq=7]', +}; + +function response(body: unknown): Response { + return new Response(JSON.stringify(body), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); +} + +function clientWith( + responseBody: unknown, + onSaveAdvisory?: (ev: MetadataSaveAdvisoryEvent) => void, +) { + return new MetadataClient({ + baseUrl: 'http://test.local', + fetch: vi.fn(async () => response(responseBody)) as unknown as typeof fetch, + ...(onSaveAdvisory ? { onSaveAdvisory } : {}), + }); +} + +/** + * The premise, asserted rather than assumed — and asserted against the + * INSTALLED `@objectstack/spec`, not against the upstream PR's description of + * it. This card was held 13 days on a spec-pin condition and released on a + * version-ordering argument; the reading that actually opened the gate is this + * one, so it lives in CI instead of in a transcript. + * + * The reverse probe is what makes the positive a measurement: a half-shaped + * finding must be REJECTED, or "the key parses" would prove only that the + * schema ignores it. + */ +describe('the contract this renders (objectstack#9176), read off the installed spec', () => { + it('declares `advisories` on the publish response, and validates its elements', () => { + const withAdvisories = PublishMetaItemResponseSchema.safeParse({ + ...CLEAN_BODY, + advisories: [PURGE_ADVISORY], + }); + expect(withAdvisories.success).toBe(true); + // Declared, not merely tolerated: an undeclared key is STRIPPED by the + // object schema, so surviving the parse is the discriminating reading. + expect( + withAdvisories.success && + Object.prototype.hasOwnProperty.call(withAdvisories.data, 'advisories'), + ).toBe(true); + + const halfShaped = PublishMetaItemResponseSchema.safeParse({ + ...CLEAN_BODY, + advisories: [{ rule: 'flow/delete-without-filter' }], + }); + expect(halfShaped.success).toBe(false); + }); + + it('omits the key entirely on a clean publish — absence means "nothing to report"', () => { + const clean = PublishMetaItemResponseSchema.safeParse(CLEAN_BODY); + expect(clean.success).toBe(true); + expect(clean.success && Object.prototype.hasOwnProperty.call(clean.data, 'advisories')).toBe( + false, + ); + }); +}); + +describe('MetadataClient.publish — runtime authoring gate advisories (#5026)', () => { + it('emits the findings a successful promotion returned', async () => { + const events: MetadataSaveAdvisoryEvent[] = []; + const client = clientWith({ ...CLEAN_BODY, advisories: [PURGE_ADVISORY] }, (e) => + events.push(e), + ); + + await client.publish('flow', 'nightly_purge'); + + expect(events).toHaveLength(1); + expect(events[0]).toEqual({ + type: 'flow', + name: 'nightly_purge', + door: 'publish', + mode: 'publish', + advisories: [PURGE_ADVISORY], + }); + }); + + it('names the PUBLISH door, which `mode` alone cannot say', async () => { + // A direct active save and a draft promotion both land the body in the + // active overlay, so both report `mode: 'publish'`. Only `door` separates + // them, and the renderer needs that separation: "Saved" after a Publish + // tells the author their change is still a draft. + const events: MetadataSaveAdvisoryEvent[] = []; + const client = clientWith({ ...CLEAN_BODY, advisories: [PURGE_ADVISORY] }, (e) => + events.push(e), + ); + + await client.publish('view', 'cases'); + + expect(events[0]!.door).toBe('publish'); + expect(events[0]!.mode).toBe('publish'); + }); + + it('carries rule, message and hint through verbatim — they are server prose', async () => { + const events: MetadataSaveAdvisoryEvent[] = []; + const client = clientWith({ ...CLEAN_BODY, advisories: [PURGE_ADVISORY] }, (e) => + events.push(e), + ); + + await client.publish('flow', 'nightly_purge'); + + const [finding] = events[0]!.advisories; + expect(finding!.rule).toBe('flow/delete-without-filter'); + expect(finding!.message).toBe(PURGE_ADVISORY.message); + expect(finding!.hint).toBe(PURGE_ADVISORY.hint); + // Never `error` on this channel — an error-severity finding refuses the + // promotion and arrives as the 422 `invalid_metadata` envelope instead. + expect(finding!.severity).toBe('warning'); + }); + + it('says nothing on a clean publish — the server omits the key entirely', async () => { + const events: MetadataSaveAdvisoryEvent[] = []; + const client = clientWith(CLEAN_BODY, (e) => events.push(e)); + + await client.publish('object', 'account'); + + expect(events).toEqual([]); + }); + + it('says nothing when the array is present but empty', async () => { + const events: MetadataSaveAdvisoryEvent[] = []; + const client = clientWith({ ...CLEAN_BODY, advisories: [] }, (e) => events.push(e)); + + await client.publish('object', 'account'); + + expect(events).toEqual([]); + }); + + it('drops half-shaped findings rather than rendering blanks at the author', async () => { + const events: MetadataSaveAdvisoryEvent[] = []; + const client = clientWith( + { ...CLEAN_BODY, advisories: [PURGE_ADVISORY, { rule: 'only-a-rule' }, null] }, + (e) => events.push(e), + ); + + await client.publish('flow', 'nightly_purge'); + + expect(events[0]!.advisories).toEqual([PURGE_ADVISORY]); + }); + + it('still returns the publish response unchanged', async () => { + const body = { ...CLEAN_BODY, advisories: [PURGE_ADVISORY] }; + const client = clientWith(body, () => {}); + + const result = await client.publish('flow', 'nightly_purge'); + + expect(result).toEqual(body); + }); + + it('a throwing sink never fails a promotion the server already committed', async () => { + const client = clientWith({ ...CLEAN_BODY, advisories: [PURGE_ADVISORY] }, () => { + throw new Error('renderer exploded'); + }); + + await expect(client.publish('flow', 'nightly_purge')).resolves.toBeTruthy(); + }); + + it('survives the withEnvironment clone — console clients are all env-scoped', async () => { + const events: MetadataSaveAdvisoryEvent[] = []; + const base = new MetadataClient({ + baseUrl: 'http://test.local', + fetch: vi.fn(async () => + response({ ...CLEAN_BODY, advisories: [PURGE_ADVISORY] }), + ) as unknown as typeof fetch, + onSaveAdvisory: (e) => events.push(e), + }); + + await base.withEnvironment('env_1').publish('flow', 'nightly_purge'); + + expect(events).toHaveLength(1); + }); + + it('survives the withPreviewDrafts clone', async () => { + const events: MetadataSaveAdvisoryEvent[] = []; + const base = new MetadataClient({ + baseUrl: 'http://test.local', + fetch: vi.fn(async () => + response({ ...CLEAN_BODY, advisories: [PURGE_ADVISORY] }), + ) as unknown as typeof fetch, + onSaveAdvisory: (e) => events.push(e), + }); + + await base.withPreviewDrafts(true).publish('flow', 'nightly_purge'); + + expect(events).toHaveLength(1); + }); +}); + +describe('MetadataClient.publishDraft — the same door, so the same report', () => { + it('emits the findings a by-reference promotion returned', async () => { + const events: MetadataSaveAdvisoryEvent[] = []; + const client = clientWith({ ...CLEAN_BODY, advisories: [PURGE_ADVISORY] }, (e) => + events.push(e), + ); + + await client.publishDraft('flow', 'nightly_purge'); + + expect(events).toHaveLength(1); + expect(events[0]!.door).toBe('publish'); + expect(events[0]!.advisories).toEqual([PURGE_ADVISORY]); + }); + + it('reads them through the dispatcher `{ success, data }` envelope it already unwraps', async () => { + // This method tolerates an enveloped body and returns the inner object, so + // it must read the advisories from the same place — otherwise the report + // would depend on which of two equivalent server shapes answered. + const events: MetadataSaveAdvisoryEvent[] = []; + const client = clientWith( + { success: true, data: { ...CLEAN_BODY, advisories: [PURGE_ADVISORY] } }, + (e) => events.push(e), + ); + + const result = await client.publishDraft('flow', 'nightly_purge'); + + expect(events).toHaveLength(1); + expect(events[0]!.advisories).toEqual([PURGE_ADVISORY]); + // and the unwrapping itself is unchanged + expect((result as { seq?: number }).seq).toBe(7); + }); + + it('says nothing on a clean by-reference publish', async () => { + const events: MetadataSaveAdvisoryEvent[] = []; + const client = clientWith(CLEAN_BODY, (e) => events.push(e)); + + await client.publishDraft('object', 'account'); + + expect(events).toEqual([]); + }); + + /** + * The scope control, and it is a real one rather than a restatement. + * + * "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). + * + * 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. + */ + it('does NOT render advisories buried in a batch-shaped `published[]` body', async () => { + const events: MetadataSaveAdvisoryEvent[] = []; + const client = clientWith( + { + success: true, + publishedCount: 1, + failedCount: 0, + published: [{ type: 'flow', name: 'nightly_purge', advisories: [PURGE_ADVISORY] }], + }, + (e) => events.push(e), + ); + + await client.publishDraft('flow', 'nightly_purge'); + + expect(events).toEqual([]); + }); +}); diff --git a/packages/data-objectstack/src/metadata-client.saveAdvisories.test.ts b/packages/data-objectstack/src/metadata-client.saveAdvisories.test.ts index c861d4204f..37af97dc70 100644 --- a/packages/data-objectstack/src/metadata-client.saveAdvisories.test.ts +++ b/packages/data-objectstack/src/metadata-client.saveAdvisories.test.ts @@ -73,6 +73,10 @@ describe('MetadataClient.save — runtime authoring gate advisories (#4133)', () expect(events[0]).toEqual({ type: 'flow', name: 'nightly_purge', + // #5026 — the event now names its door, and this is the SAVE one. The + // whole-object assertion is deliberate: it is what would catch the + // publish door's `door: 'publish'` leaking into a save. + door: 'save', mode: 'publish', advisories: [PURGE_ADVISORY], }); diff --git a/packages/data-objectstack/src/metadata-client.ts b/packages/data-objectstack/src/metadata-client.ts index 2bc096aa17..83af5ebd4e 100644 --- a/packages/data-objectstack/src/metadata-client.ts +++ b/packages/data-objectstack/src/metadata-client.ts @@ -57,15 +57,31 @@ import type { export type { RuntimeAuthoringIssue }; /** - * Emitted after a {@link MetadataClient.save} whose response carried a - * non-empty `advisories` array. The save SUCCEEDED — the row persisted and the - * server returned 200 — so this is advisory, never a failure. + * Emitted after a metadata WRITE whose response carried a non-empty + * `advisories` array. The write SUCCEEDED — the row persisted and the server + * returned 200 — so this is advisory, never a failure. * * Deliberately the same shape of seam as `ObjectStackAdapter.onWriteWarning` * (#3431/#3455): a successful write whose response carries something the author * needs to be told, surfaced to the shell as an event so the data layer never * imports a toaster. The difference is only which door produced it — that one - * is record CRUD, this one is the metadata save door. + * is record CRUD, these are the metadata write doors. + * + * ## Both write doors report (#5026) + * + * The #4463 runtime authoring gate runs on BOTH metadata write doors by its D1 + * ruling — a draft→active promotion is gated exactly as a direct active save — + * and its non-blocking findings ride the 2xx of whichever door earned them. + * `SaveMetaItemResponseSchema` has carried the key since #4717 and + * `PublishMetaItemResponseSchema` since objectstack#9176; both declare it at + * the response's TOP level, under the same name and with the same + * `RuntimeAuthoringIssueSchema` element type, which is what lets one reader and + * one event serve both. Measured against the installed `@objectstack/spec` + * rather than assumed — see the PR for the probe. + * + * 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. */ export interface MetadataSaveAdvisoryEvent { /** Metadata type saved (e.g. `'flow'`). */ @@ -80,6 +96,27 @@ export interface MetadataSaveAdvisoryEvent { * consumer reading the event can tell which door it came through. */ mode: 'draft' | 'publish'; + /** + * Which write door produced this event (#5026). + * + * - `'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}). + * + * Distinct from {@link mode}, which cannot answer this: a direct active save + * and a draft promotion both report `mode: 'publish'` because both land the + * body in the active overlay. The renderer needs the DOOR, because the author + * pressed either Save or Publish and a toast that says "Saved" after a + * Publish tells them their change is still a draft — the opposite of what + * happened, and in this product Save and Publish are two different buttons + * with two different meanings. + * + * REQUIRED rather than optional-with-a-default so a future third door cannot + * be wired without stating which one it is: an omitted discriminator would + * silently render the save wording. + */ + door: 'save' | 'publish'; /** The findings. Never empty — the event is not emitted otherwise. */ advisories: RuntimeAuthoringIssue[]; } @@ -88,9 +125,14 @@ export interface MetadataSaveAdvisoryEvent { export type MetadataSaveAdvisoryListener = (event: MetadataSaveAdvisoryEvent) => void; /** - * Read the `advisories` array off a save response, defensively. + * 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. * - * The server omits the key entirely on a clean save, so `undefined` is the + * 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 * objects carrying the six required keys is dropped rather than rendered: a * half-shaped finding would print blanks at the author, and this channel must @@ -139,7 +181,9 @@ export interface MetadataClientConfig { previewDrafts?: boolean; /** * Called after a {@link MetadataClient.save} whose 2xx response carried a - * non-empty `advisories` array (objectstack#7435). The save already + * 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. * @@ -637,7 +681,7 @@ export class MetadataClient { private readonly headers: Record; /** ADR-0037: when true, reads render the draft-overlaid world. */ readonly previewDrafts: boolean; - /** #4133 — sink for post-save advisory findings; see the config field. */ + /** #4133 / #5026 — sink for both write doors' advisory findings; see the config field. */ private readonly onSaveAdvisory: MetadataSaveAdvisoryListener | undefined; constructor(config: MetadataClientConfig) { @@ -840,6 +884,33 @@ export class MetadataClient { }); } + /** + * Report the runtime authoring gate's advisory findings for a write the + * server has ALREADY committed (#4133 for the save door, #5026 for the + * publish door). + * + * Everything here is best-effort by construction, and that is the whole + * contract: the row is committed server-side before this runs, so neither a + * malformed body, nor a missing key, nor a throwing sink may change what the + * 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. + */ + private emitAdvisories( + body: unknown, + event: Omit, + ): void { + if (!this.onSaveAdvisory) return; + try { + const advisories = readSaveAdvisories(body); + if (advisories.length > 0) this.onSaveAdvisory({ ...event, advisories }); + } catch { + /* an advisory must never turn a committed write into a thrown error */ + } + } + /** * Save (PUT) a metadata item. The framework accepts both the bare * item payload and the `{ item: ... }` / `{ metadata: ... }` @@ -880,25 +951,12 @@ export class MetadataClient { if (!res.ok) throw await parseError(res); const body = await res.json(); // #4133 — the runtime authoring gate's advisory findings (objectstack#7435). - // The server emits `advisories` ONLY when non-empty, so a clean save costs - // nothing here. Everything below is best-effort by construction: the save - // has already been committed server-side and returning it must not depend - // on anything the advisory channel does. - if (this.onSaveAdvisory) { - try { - const advisories = readSaveAdvisories(body); - if (advisories.length > 0) { - this.onSaveAdvisory({ - type, - name, - mode: options.mode === 'draft' ? 'draft' : 'publish', - advisories, - }); - } - } catch { - /* an advisory must never turn a committed save into a thrown error */ - } - } + this.emitAdvisories(body, { + type, + name, + door: 'save', + mode: options.mode === 'draft' ? 'draft' : 'publish', + }); return body as T; } @@ -913,6 +971,12 @@ export class MetadataClient { * the rows and reports under `seedApplied` — a data problem never fails the * publish, so callers should check `seedApplied?.success` and warn the user * 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. */ async publishDraft( type: string, @@ -931,6 +995,11 @@ export class MetadataClient { const body = (await res.json().catch(() => ({}))) as Record; // Tolerate the dispatcher's `{ success, data: {...} }` envelope. const inner = (body as any)?.data && typeof (body as any).data === 'object' ? (body as any).data : body; + // #5026 — this is the SAME single-item publish door `publish()` uses, so it + // reports the same way. Read off `inner`, the object this method returns: + // `advisories` is declared at the top level of `PublishMetaItemResponse`, + // which is what `inner` is once the dispatcher envelope (if any) is off. + this.emitAdvisories(inner, { type, name, door: 'publish', mode: 'publish' }); return inner as any; } @@ -1151,6 +1220,12 @@ export class MetadataClient { * `{ success, version, seq, message }`. Throws a `404 no_draft` if * nothing is pending and `409 metadata_conflict` if the published * overlay moved while the draft was sitting. + * + * Reports the runtime authoring gate's advisory findings through + * {@link MetadataClientConfig.onSaveAdvisory} when the response carries them + * (#5026 / objectstack#9176), with `door: 'publish'`. This is the door the + * Studio designer takes on every edit, so it is the one whose findings an + * author actually sees. */ async publish( type: string, @@ -1182,7 +1257,18 @@ export class MetadataClient { body: JSON.stringify(options.message ? { message: options.message } : {}), }); if (!res.ok) throw await parseError(res); - return (await res.json()) as T; + const body = await res.json(); + // #5026 — the runtime authoring gate's advisory findings on the publish + // door (objectstack#9176). This is the door Studio's designer takes on + // every edit: it saves a draft (never gated — the gate's D1 early-return + // fires before a single rule runs) and then promotes it HERE, which is the + // write the gate actually grades. Wiring only the save door therefore left + // the one flow most tenants use silent, which is what this closes. + // + // `mode` is `'publish'` because the promotion lands the body in the active + // overlay; `door` is what distinguishes it from a direct active save. + this.emitAdvisories(body, { type, name, door: 'publish', mode: 'publish' }); + return body as T; } /** diff --git a/packages/data-objectstack/src/onSaveAdvisory.test.ts b/packages/data-objectstack/src/onSaveAdvisory.test.ts index cbb1b1e401..be12bbba13 100644 --- a/packages/data-objectstack/src/onSaveAdvisory.test.ts +++ b/packages/data-objectstack/src/onSaveAdvisory.test.ts @@ -96,6 +96,9 @@ describe('ObjectStackAdapter.onSaveAdvisory — the second client class (#4237)' { type: 'flow', name: 'nightly_purge', + // #5026 — this interceptor wraps the SDK's `meta.saveItem`, so every + // event it emits comes through the save door by construction. + door: 'save', mode: 'publish', advisories: [PURGE_ADVISORY], }, diff --git a/packages/i18n/src/locales/ar.ts b/packages/i18n/src/locales/ar.ts index a7786651c0..0fe45d0950 100644 --- a/packages/i18n/src/locales/ar.ts +++ b/packages/i18n/src/locales/ar.ts @@ -1389,6 +1389,7 @@ const ar = { }, console: { saveAdvisoryTitle: "تم الحفظ — أنتج فحص التأليف {{count}} ملاحظة إرشادية", + publishAdvisoryTitle: "تم النشر — أنتج فحص التأليف {{count}} ملاحظة إرشادية", settingsHub: { title: "الإعدادات", subtitle: "اضبط مساحة العمل والتكاملات وأعلام الميزات.", diff --git a/packages/i18n/src/locales/de.ts b/packages/i18n/src/locales/de.ts index 574778d555..77a20f086e 100644 --- a/packages/i18n/src/locales/de.ts +++ b/packages/i18n/src/locales/de.ts @@ -1382,6 +1382,7 @@ const de = { }, console: { saveAdvisoryTitle: "Gespeichert — die Autorenprüfung ergab {{count}} Hinweis(e)", + publishAdvisoryTitle: "Veröffentlicht — die Autorenprüfung ergab {{count}} Hinweis(e)", settingsHub: { title: "Einstellungen", subtitle: "Konfigurieren Sie Ihren Workspace, Integrationen und Feature-Flags.", diff --git a/packages/i18n/src/locales/en.ts b/packages/i18n/src/locales/en.ts index e61f7f07a5..6adf36eef4 100644 --- a/packages/i18n/src/locales/en.ts +++ b/packages/i18n/src/locales/en.ts @@ -1576,6 +1576,7 @@ const en = { }, console: { saveAdvisoryTitle: 'Saved — the authoring check raised {{count}} advisory finding(s)', + publishAdvisoryTitle: 'Published — the authoring check raised {{count}} advisory finding(s)', title: 'ObjectOS', initializing: 'Initializing application…', search: 'Search…', diff --git a/packages/i18n/src/locales/es.ts b/packages/i18n/src/locales/es.ts index edf462e1a5..c97ceaa167 100644 --- a/packages/i18n/src/locales/es.ts +++ b/packages/i18n/src/locales/es.ts @@ -1386,6 +1386,7 @@ const es = { }, console: { saveAdvisoryTitle: "Guardado: la comprobación de creación generó {{count}} recomendación(es)", + publishAdvisoryTitle: "Publicado: la comprobación de creación generó {{count}} recomendación(es)", settingsHub: { title: "Configuración", subtitle: "Configure su espacio de trabajo, las integraciones y los indicadores de funciones.", diff --git a/packages/i18n/src/locales/fr.ts b/packages/i18n/src/locales/fr.ts index 99b90ae7e5..45b3a5c24a 100644 --- a/packages/i18n/src/locales/fr.ts +++ b/packages/i18n/src/locales/fr.ts @@ -1384,6 +1384,7 @@ const fr = { }, console: { saveAdvisoryTitle: "Enregistré — le contrôle de création a signalé {{count}} recommandation(s)", + publishAdvisoryTitle: "Publié — le contrôle de création a signalé {{count}} recommandation(s)", settingsHub: { title: "Paramètres", subtitle: "Configurez votre espace de travail, vos intégrations et vos indicateurs de fonctionnalité.", diff --git a/packages/i18n/src/locales/ja.ts b/packages/i18n/src/locales/ja.ts index 5b634d3b42..3417fa379a 100644 --- a/packages/i18n/src/locales/ja.ts +++ b/packages/i18n/src/locales/ja.ts @@ -1382,6 +1382,7 @@ const ja = { }, console: { saveAdvisoryTitle: "保存しました — 編集チェックで {{count}} 件の推奨事項が見つかりました", + publishAdvisoryTitle: "公開しました — 編集チェックで {{count}} 件の推奨事項が見つかりました", settingsHub: { title: "設定", subtitle: "ワークスペース、連携、機能フラグを設定します。", diff --git a/packages/i18n/src/locales/ko.ts b/packages/i18n/src/locales/ko.ts index c5ba4cee91..3fef822442 100644 --- a/packages/i18n/src/locales/ko.ts +++ b/packages/i18n/src/locales/ko.ts @@ -1382,6 +1382,7 @@ const ko = { }, console: { saveAdvisoryTitle: "저장되었습니다 — 작성 검사에서 {{count}}건의 권장 사항이 발견되었습니다", + publishAdvisoryTitle: "게시되었습니다 — 작성 검사에서 {{count}}건의 권장 사항이 발견되었습니다", settingsHub: { title: "설정", subtitle: "워크스페이스, 연동, 기능 플래그를 구성합니다.", diff --git a/packages/i18n/src/locales/pt.ts b/packages/i18n/src/locales/pt.ts index e5c661af80..9258def3df 100644 --- a/packages/i18n/src/locales/pt.ts +++ b/packages/i18n/src/locales/pt.ts @@ -1381,6 +1381,7 @@ const pt = { }, console: { saveAdvisoryTitle: "Salvo — a verificação de criação gerou {{count}} recomendação(ões)", + publishAdvisoryTitle: "Publicado — a verificação de criação gerou {{count}} recomendação(ões)", settingsHub: { title: "Configurações", subtitle: "Configure seu workspace, integrações e sinalizadores de recursos.", diff --git a/packages/i18n/src/locales/ru.ts b/packages/i18n/src/locales/ru.ts index 3525b3916a..016124fc84 100644 --- a/packages/i18n/src/locales/ru.ts +++ b/packages/i18n/src/locales/ru.ts @@ -1392,6 +1392,7 @@ const ru = { }, console: { saveAdvisoryTitle: "Сохранено — проверка авторинга выдала рекомендаций: {{count}}", + publishAdvisoryTitle: "Опубликовано — проверка авторинга выдала рекомендаций: {{count}}", settingsHub: { title: "Настройки", subtitle: "Настройте рабочее пространство, интеграции и флаги функций.", diff --git a/packages/i18n/src/locales/zh.ts b/packages/i18n/src/locales/zh.ts index 7c14147010..b4fb43c26c 100644 --- a/packages/i18n/src/locales/zh.ts +++ b/packages/i18n/src/locales/zh.ts @@ -1447,6 +1447,7 @@ const zh = { }, console: { saveAdvisoryTitle: '已保存 — 编辑检查提出了 {{count}} 条建议', + publishAdvisoryTitle: '已发布 — 编辑检查提出了 {{count}} 条建议', title: 'ObjectStack 控制台', initializing: '正在初始化应用程序…', search: '搜索…',