feat(data): surface save advisories from the second metadata client class (#4237)

.changeset/second-client-save-advisories-4237.md

@@ -0,0 +1,14 @@
+---
+'@object-ui/data-objectstack': patch
+'@object-ui/app-shell': patch
+---
+
+The second metadata client class surfaces the runtime authoring gate's advisories instead of discarding them
+
+objectui#4133 (PR #4236) put the gate's advisory findings — the ones that ride a **200**, where the save succeeded and the row persisted — in front of Studio authors, but it covered only one of the two client classes that write through `PUT /api/v1/meta/:type/:name`. The wiring lifts at `useMetadataClient`, which is where every app-shell path takes its `MetadataClient` from. `ObjectStackClient.meta.saveItem` — the SDK client hanging off `ObjectStackAdapter` — is a different class reaching the same door, and every one of its callers awaited the call and discarded the response, so an `advisories[]` the server attached was parsed off the wire and dropped one layer further out.
+
+Those callers all write in **active** mode, so this is not the draft case where the gate never runs: the gate does run for them, produces findings, and the author was told nothing. The list is `MetadataService` (five saves behind the Object Manager and Field Designer), `useNavigationSync`, plugin-designer's Create/EditAppPage, and the adapter's own `updateViewConfig` / view / `updateDashboard` paths.
+
+`ObjectStackAdapter` now carries an `onSaveAdvisory(listener)` subscription and emits on it after a metadata save whose 200 carried a non-empty `advisories[]`; `AdapterProvider` subscribes once and renders through the same `emitSaveAdvisories` the other client class already uses, so both doors produce one wording on the warning tier that says "Saved" first. The emitter is installed **once at the adapter/client seam** rather than at the call sites: every caller above reaches the save door through the adapter's own long-lived `ObjectStackClient`, so one interception covers all of them, plus any future one, without a toast copied into a dozen places — the same reasoning that put #4133's sink at one factory instead of twenty call sites.
+
+It is a sibling of the `onWriteWarning` channel (#3431/#3455) rather than a second payload pushed down it, which is what `MetadataSaveAdvisoryEvent` already said it was modelled on. `WriteWarningEvent` is a closed shape whose required `droppedFields` means "fields the write legally stripped", so carrying advisories on it would either force every existing subscriber to grow a branch or make the event lie about what happened. The seam's shape is reused; its event type is not. `readSaveAdvisories` is shared unchanged between the two clients — one reader, two call sites — which the response envelopes make possible: the spec puts `advisories` at the save body's top level, and the SDK returns that body verbatim (it strips its `{ success, data }` envelope only when a `data` key is present, and this body has none). That measurement is pinned by tests that drive a real SDK client through a fake `fetch` rather than stubbing the method under test.

packages/app-shell/src/providers/AdapterProvider.tsx

@@ -15,6 +15,7 @@ import { AdapterCtx } from '@object-ui/react';
 import { useObjectTranslation, useSafeFieldLabel } from '@object-ui/i18n';
 import { installSettleSignalGlobal, withSettleSignal } from '../observability/settleSignal';
 import { emitWriteWarning, type TranslateFn } from './writeWarningToast';
+import { emitSaveAdvisories } from './saveAdvisoryToast';
 
 export { useAdapter } from '@object-ui/react';
 
@@ -52,6 +53,7 @@ export function AdapterProvider({ children, adapter: externalAdapter }: AdapterP
 
     let cancelled = false;
     let unsubscribeWriteWarning: (() => void) | undefined;
+    let unsubscribeSaveAdvisory: (() => void) | undefined;
 
     // Expose window.__objectui.{pendingRequests,idle,whenIdle} so an automated
     // (AI) browser driver has one "is the app settled?" predicate (ADR-0054 C5).
@@ -77,6 +79,19 @@ export function AdapterProvider({ children, adapter: externalAdapter }: AdapterP
           void emitWriteWarning(ev, tRef.current as TranslateFn, a, fieldLabelRef.current, toast);
         });
 
+        // Surface the runtime authoring gate's advisory findings for metadata
+        // saves that went through THIS adapter's `ObjectStackClient.meta`
+        // (#4237) — `MetadataService`, `useNavigationSync`, plugin-designer's
+        // app wizard, and the adapter's own view/dashboard save paths all take
+        // that client from `getClient()`, so this one subscription covers every
+        // one of them. The renderer is the same `emitSaveAdvisories` the other
+        // client class already uses (#4133/#4236): one wording, two doors. `t`
+        // rides the same ref as the write-warning channel above, and for the
+        // same reason — the adapter outlives a language switch.
+        unsubscribeSaveAdvisory = a.onSaveAdvisory((ev) => {
+          emitSaveAdvisories(ev, tRef.current as TranslateFn, toast);
+        });
+
         await a.connect();
 
         if (!cancelled) {
@@ -93,6 +108,7 @@ export function AdapterProvider({ children, adapter: externalAdapter }: AdapterP
     return () => {
       cancelled = true;
       unsubscribeWriteWarning?.();
+      unsubscribeSaveAdvisory?.();
     };
   }, [externalAdapter]);
 

packages/app-shell/src/services/MetadataService.saveAdvisories.test.ts

@@ -0,0 +1,155 @@
+/**
+ * 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.
+ */
+
+/**
+ * End-to-end pin for objectui#4237: a save through the SECOND client class
+ * reaches the shell's warning surface.
+ *
+ * `MetadataService` is the most user-reachable of the callers #4237 enumerates
+ * — it is what the Object Manager and Field Designer persist through, five save
+ * sites in one class — and it is deliberately NOT edited by that fix. That is
+ * the claim under test: the emitter sits at the adapter/client seam, so a caller
+ * that says nothing about advisories still gets them rendered.
+ *
+ * The chain exercised here, with nothing stubbed in the middle:
+ *
+ *   MetadataService.saveObject
+ *     → adapter.getClient().meta.saveItem          (the enumerated call site)
+ *       → the SDK's real PUT + `unwrapResponse`    (fake `fetch` answers 200)
+ *         → the adapter's save-advisory interceptor
+ *           → adapter.onSaveAdvisory subscribers   (what AdapterProvider wires)
+ *             → emitSaveAdvisories                 (the #4133/#4236 renderer)
+ *               → the warning tier
+ *
+ * `AdapterProvider` is what wires the last two links in the real app; the sink
+ * is handed over here instead of mounting a toaster, exactly as
+ * `saveAdvisoryToast.test.ts` does, so nothing depends on module mocking.
+ */
+
+import { describe, it, expect, vi } from 'vitest';
+import { ObjectStackAdapter, type MetadataSaveAdvisoryEvent } from '@object-ui/data-objectstack';
+import type { ObjectDefinition } from '@object-ui/types';
+import { MetadataService } from './MetadataService';
+import { emitSaveAdvisories, type TranslateFn } from '../providers/saveAdvisoryToast';
+
+const PURGE_ADVISORY = {
+  severity: 'warning' as const,
+  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',
+};
+
+const CLEAN_BODY = { success: true, version: 'v2', seq: 4, state: 'active' as const };
+
+/** i18next-shaped `t` that renders the inline default with its holes filled. */
+const t: TranslateFn = (key, options) => {
+  const raw = (options?.defaultValue as string) ?? key;
+  let out = raw;
+  for (const [k, v] of Object.entries(options ?? {})) {
+    if (k === 'defaultValue') continue;
+    out = out.split(`{{${k}}}`).join(String(v));
+  }
+  return out;
+};
+
+function makeSink() {
+  return {
+    warning: vi.fn<(title: string, opts?: { description?: string; duration?: number }) => void>(),
+    // Present so a mistaken `sink.error(...)` is observable rather than a
+    // TypeError — the assertion below is that it is never reached.
+    error: vi.fn(),
+    success: vi.fn(),
+  };
+}
+
+/**
+ * A real adapter answering every metadata PUT with `body`, with the shell's
+ * `AdapterProvider` wiring reproduced: one `onSaveAdvisory` subscription that
+ * renders through `emitSaveAdvisories` into a caller-owned sink.
+ */
+function makeWiredAdapter(body: unknown) {
+  const adapter = new ObjectStackAdapter({
+    baseUrl: 'http://test.local',
+    fetch: vi.fn(async () =>
+      new Response(JSON.stringify(body), {
+        status: 200,
+        headers: { 'content-type': 'application/json' },
+      }),
+    ) as unknown as typeof fetch,
+  });
+  const sink = makeSink();
+  const events: MetadataSaveAdvisoryEvent[] = [];
+  adapter.onSaveAdvisory((ev) => {
+    events.push(ev);
+    emitSaveAdvisories(ev, t, sink);
+  });
+  return { adapter, sink, events };
+}
+
+const ACCOUNT: ObjectDefinition = { name: 'account', label: 'Account', fields: [] } as ObjectDefinition;
+
+describe('MetadataService saves reach the shell advisory surface (#4237)', () => {
+  it('renders the gate findings for a save that succeeded', async () => {
+    const { adapter, sink, events } = makeWiredAdapter({
+      ...CLEAN_BODY,
+      advisories: [PURGE_ADVISORY],
+    });
+
+    await new MetadataService(adapter).saveObject(ACCOUNT);
+
+    expect(events).toHaveLength(1);
+    expect(events[0]).toMatchObject({ type: 'object', name: 'account', mode: 'publish' });
+    expect(sink.warning).toHaveBeenCalledTimes(1);
+  });
+
+  it('lands on the WARNING tier and says "Saved" first — the write succeeded', async () => {
+    const { adapter, sink } = makeWiredAdapter({ ...CLEAN_BODY, advisories: [PURGE_ADVISORY] });
+
+    await new MetadataService(adapter).saveObject(ACCOUNT);
+
+    const [title, opts] = sink.warning.mock.calls[0]!;
+    expect(title).toMatch(/^Saved/);
+    expect(sink.error).not.toHaveBeenCalled();
+    // Server prose, rendered verbatim.
+    expect(opts!.description).toContain(PURGE_ADVISORY.message);
+    expect(opts!.description).toContain(PURGE_ADVISORY.hint);
+  });
+
+  it('a clean save renders no new UI', async () => {
+    const { adapter, sink, events } = makeWiredAdapter(CLEAN_BODY);
+
+    await new MetadataService(adapter).saveObject(ACCOUNT);
+
+    expect(events).toEqual([]);
+    expect(sink.warning).not.toHaveBeenCalled();
+    expect(sink.error).not.toHaveBeenCalled();
+    expect(sink.success).not.toHaveBeenCalled();
+  });
+
+  it("covers the service's generic save door too, not just saveObject", async () => {
+    const { adapter, sink } = makeWiredAdapter({ ...CLEAN_BODY, advisories: [PURGE_ADVISORY] });
+
+    await new MetadataService(adapter).saveMetadataItem('flow', 'nightly_purge', {
+      name: 'nightly_purge',
+    });
+
+    expect(sink.warning).toHaveBeenCalledTimes(1);
+  });
+
+  it('still performs the save — the advisory channel changes nothing about it', async () => {
+    const { adapter } = makeWiredAdapter({ ...CLEAN_BODY, advisories: [PURGE_ADVISORY] });
+    const invalidate = vi.spyOn(adapter, 'invalidateCache');
+
+    await expect(new MetadataService(adapter).saveObject(ACCOUNT)).resolves.toBeUndefined();
+
+    // The service's own post-save step still runs.
+    expect(invalidate).toHaveBeenCalledWith('object:account');
+  });
+});

packages/data-objectstack/src/index.ts

@@ -8,6 +8,14 @@
 
 import { ObjectStackClient, type QueryOptions as ObjectStackQueryOptions } from '@objectstack/client';
 import type { DroppedFieldsEvent } from '@objectstack/spec/data';
+// #4237 — the metadata save door's advisory reader, shared with `MetadataClient`
+// rather than forked. ONE reader, two call sites: the other client class calls it
+// from `MetadataClient.save` (#4133/#4236), this one from the interceptor below.
+import {
+  readSaveAdvisories,
+  type MetadataSaveAdvisoryEvent,
+  type MetadataSaveAdvisoryListener,
+} from './metadata-client';
 import type { AnalyticsResult, DatasetSelection } from '@objectstack/spec/contracts';
 import type {
   DataSource,
@@ -1053,6 +1061,13 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
   // shell can surface a toast instead of the strip passing silently.
   private writeWarningListeners = new Set<WriteWarningListener>();
 
+  // Subscribers registered via onSaveAdvisory(). Emitted after a metadata save
+  // through THIS adapter's `ObjectStackClient` whose 200 carried a non-empty
+  // `advisories` array (#4237; backend objectstack#7435). Sibling of the set
+  // above in every respect except which door produced the event: that one is
+  // record CRUD, this one is the metadata save door.
+  private saveAdvisoryListeners = new Set<MetadataSaveAdvisoryListener>();
+
   constructor(config: {
     baseUrl: string;
     token?: string;
@@ -1070,6 +1085,9 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
     // debug() so they don't pollute the browser console. Other log levels are
     // forwarded to the standard console.
     this.client = new ObjectStackClient({ ...config, logger: createQuietHttpLogger() });
+    // #4237 — one emitter for every metadata save this adapter's client makes,
+    // installed the moment the client exists so no save can precede it.
+    this.installSaveAdvisoryInterceptor();
     this.metadataCache = new MetadataCache(config.cache);
     this.autoReconnect = config.autoReconnect ?? true;
     this.maxReconnectAttempts = config.maxReconnectAttempts ?? 3;
@@ -1585,6 +1603,122 @@ export class ObjectStackAdapter<T = unknown> implements DataSource<T> {
     };
   }
 
+  /**
+   * Subscribe to metadata save-advisory events — the runtime authoring gate's
+   * advisory findings on a save that SUCCEEDED (#4237; backend
+   * objectstack#7435). Returns an unsubscribe function.
+   *
+   * Deliberately the same seam as {@link onWriteWarning} (#3431/#3455), which
+   * is what {@link MetadataSaveAdvisoryEvent}'s own declaration already said it
+   * was modelled on. It is a SIBLING of that channel rather than a second
+   * payload pushed down it: `WriteWarningEvent` is a closed shape whose
+   * `droppedFields` is required and means "fields the write legally stripped",
+   * so carrying advisories on it would either force every existing
+   * `onWriteWarning` consumer to grow a branch or make the event lie about what
+   * happened. The seam's SHAPE is what is reused here — a long-lived instance
+   * with a `subscribe → unsubscribe` registration that `AdapterProvider` wires
+   * once — not its event type.
+   *
+   * Why here and not on the config, which is how the other client class does it
+   * (#4133/#4236): `MetadataClient` is minted per component by
+   * `useMetadataClient`, so it has no instance to subscribe to and its sink
+   * rides the factory. `ObjectStackAdapter` is the opposite — one long-lived
+   * instance per app, already carrying this exact subscription pattern.
+   */
+  onSaveAdvisory(callback: MetadataSaveAdvisoryListener): () => void {
+    this.saveAdvisoryListeners.add(callback);
+    return () => {
+      this.saveAdvisoryListeners.delete(callback);
+    };
+  }
+
+  /**
+   * Notify all save-advisory subscribers. Isolated exactly like
+   * {@link emitWriteWarning}: a throwing listener must neither break the save
+   * nor starve the others.
+   */
+  private emitSaveAdvisory(event: MetadataSaveAdvisoryEvent): void {
+    for (const listener of this.saveAdvisoryListeners) {
+      try {
+        listener(event);
+      } catch (err) {
+        console.warn('ObjectStackAdapter: save-advisory listener error', err);
+      }
+    }
+  }
+
+  /**
+   * Install the ONE emitter for the metadata save door (#4237).
+   *
+   * ## Why this seam, and what it covers
+   *
+   * `ObjectStackClient.meta.saveItem` is the second client class that writes
+   * through `PUT /api/v1/meta/:type/:name`, and every one of its callers reaches
+   * it through an adapter this class constructed — the four inside this file
+   * (`updateViewConfig`, the two view paths, `updateDashboard`) via
+   * `this.client`, and every caller outside it via {@link getClient}, which
+   * hands back this same instance: `MetadataService` (app-shell, five saves),
+   * `useNavigationSync`, and plugin-designer's Create/EditAppPage. Wrapping the
+   * method once here therefore covers all of them WITHOUT a per-site edit, which
+   * is the whole point — a toast copied into a dozen call sites is the shape
+   * #4133 rejected for the other client class and it is no better here.
+   *
+   * `meta` is an own, writable property assigned per instance in the SDK's
+   * constructor (`this.meta = { … }`), and the client this adapter builds is
+   * never shared, so the wrap is bounded to an object this adapter owns for its
+   * whole lifetime. It is not a prototype or global patch.
+   *
+   * ## Response shape — measured, not assumed
+   *
+   * The two client classes' envelopes coincide at the top level, which is what
+   * makes `readSaveAdvisories` reusable unchanged across both. `SaveMetaItem-
+   * ResponseSchema` puts `advisories` at the body's top level next to
+   * `success` / `version` / `seq` / `state`, and the SDK's `unwrapResponse`
+   * strips its `{ success, data }` envelope only when the body actually HAS a
+   * `data` key — this body does not, so it is returned verbatim. So the same
+   * reader that `MetadataClient.save` uses reads this response correctly, and
+   * the pins in `onSaveAdvisory.test.ts` drive a real SDK client through a fake
+   * `fetch` rather than stubbing `meta`, so that continues to be measured.
+   *
+   * ## Draft-door honesty (D1)
+   *
+   * Drafts are NEVER gated: the framework returns at its D1 early-return
+   * (`if (args.state !== 'active') return null`) before running a rule, so a
+   * draft save produces no findings to withhold. This client class has no draft
+   * door at all to worry about — the SDK's `saveItem(type, name, item)` takes no
+   * mode and always writes the active door, which is exactly why the gate DOES
+   * run for its callers. `mode` on the emitted event is therefore derived from
+   * the response's own `state` rather than from a request-side flag that does
+   * not exist here: `'draft'` when the server says the row landed as a draft,
+   * `'publish'` otherwise. That keeps the event truthful about which door it
+   * came through instead of hard-coding one.
+   */
+  private installSaveAdvisoryInterceptor(): void {
+    const meta = this.client.meta;
+    const original = meta.saveItem.bind(meta);
+    meta.saveItem = async (type: string, name: string, item: any) => {
+      const result = await original(type, name, item);
+      // Everything below is best-effort by construction: the row is already
+      // committed server-side, so nothing the advisory channel does may change
+      // what this call returns or whether it throws.
+      try {
+        const advisories = readSaveAdvisories(result);
+        if (advisories.length > 0) {
+          this.emitSaveAdvisory({
+            type,
+            name,
+            mode: (result as { state?: string } | null | undefined)?.state === 'draft' ? 'draft' : 'publish',
+            advisories,
+          });
+        }
+      } catch (err) {
+        /* an advisory must never turn a committed save into a thrown error */
+        console.warn('ObjectStackAdapter: save-advisory read error', err);
+      }
+      return result;
+    };
+  }
+
   async create(resource: string, data: Partial<T>): Promise<T> {
     await this.connect();
     try {