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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions .changeset/render-save-advisory-findings-4133.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
---
'@object-ui/data-objectstack': patch
'@object-ui/app-shell': patch
'@object-ui/i18n': patch
---

Studio surfaces the runtime authoring gate's advisory findings instead of discarding them client-side

The framework's runtime authoring gate produces two kinds of verdict on a metadata write. Errors become a 422 and the author sees them. Advisories ride a **200** — the save succeeded, the row persisted, the version bumped — and until objectstack#7435 the server dropped them into a deduped `console.warn` behind a process-level set. That landing put them on the wire as an optional `advisories[]` on the save response, emitted only when non-empty, and objectui was still throwing them away one layer further out: `MetadataClient.save` parsed the body, returned it as an opaque `T`, and every call site awaited it for its side effect and discarded the value.

The measured case the fix is built on: a `nightly_purge` flow whose only defect is a `delete_record` node with `multi: true` and no filter yields `errors = 0 / advisories = 1`. The save returns 200, the flow goes live, and nothing anywhere tells the author it deletes every row. That matters most for exactly the authors Studio serves — a Studio tenant or an MCP/AI author has no `os lint` and no CLI config for `sys_metadata` overlay rows, so this gate is not the weakest of four doors, it is the only one.

`MetadataClient` now carries an `onSaveAdvisory` sink, invoked after a save whose response carried a non-empty `advisories[]`, and the console wires it in `useMetadataClient` — the one hook every app-shell write path takes its client from, so a single wiring covers `ResourceEditPage`, `StudioDesignSurface`, `EmbeddedItemEditor`, `DatasourceResourcePage`, `ObjectHooksPanel` and any future call site rather than a toast copied into twenty of them. The finding shape is re-exported from `@objectstack/spec` (`RuntimeAuthoringIssue`) rather than restated, so it cannot fork from the 422 `issues[]` it deliberately shares a declaration with.

The affordance is the warning tier and says "Saved" first. A successful save that reads as a failure is the specific defect this surface must not ship, so the toast acknowledges the write, lists `rule` + `message` + `hint` per finding with `where` as secondary context, and renders that text **verbatim** — `message` and `hint` are server prose composed by the gate's rules, not i18n keys. Only the frame around them is translated (`console.saveAdvisoryTitle`, ten packs). The sink is best-effort in both directions: a malformed finding is dropped rather than printed as blanks, and a throwing renderer cannot turn a save the server already committed into an error.

**What this does not surface yet, and why.** Studio's designer saves as a **draft** on every edit, and drafts are never gated — the framework returns at its D1 early-return (`if (args.state !== 'active') return null`) before running a single rule, so a draft save produces no findings at all rather than producing some that get withheld. The publish step that promotes a draft to active *does* run the gate, but the publish route returns no `advisories` field until objectstack#7294 lands. So a draft-then-publish flow renders nothing today, at both of its doors, for two different reasons; the active-mode save door renders findings now. That gap is pinned as a test rather than left for a reader to rediscover.
162 changes: 162 additions & 0 deletions packages/app-shell/src/providers/saveAdvisoryToast.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
/**
* 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 post-save advisory affordance (objectui#4133).
*
* The save SUCCEEDED, so the whole point of these pins is the tier: the
* findings land on the WARNING channel and never the error one. A successful
* save that reads as a failure is the specific defect this surface must not
* ship — the card says so, and the sibling write-warning toast
* (`writeWarningToast.ts`) learned the same lesson at objectui#3484 point B.
*
* The sink is handed over rather than mocked, exactly as its sibling does, so
* nothing here depends on vitest project isolation.
*/

import { describe, it, expect, vi } from 'vitest';
import type { MetadataSaveAdvisoryEvent } from '@object-ui/data-objectstack';
import { emitSaveAdvisories, type TranslateFn } from './saveAdvisoryToast';

/** 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(...)` would be observable rather than
// a TypeError — the assertion below is that it is never reached.
error: vi.fn(),
success: vi.fn(),
};
}

const FINDING = {
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',
};

function event(overrides: Partial<MetadataSaveAdvisoryEvent> = {}): MetadataSaveAdvisoryEvent {
return {
type: 'flow',
name: 'nightly_purge',
mode: 'publish',
advisories: [FINDING],
...overrides,
};
}

describe('emitSaveAdvisories (#4133)', () => {
it('renders the findings on the WARNING channel, never the error one', () => {
const sink = makeSink();

emitSaveAdvisories(event(), t, sink);

expect(sink.warning).toHaveBeenCalledTimes(1);
expect(sink.error).not.toHaveBeenCalled();
});

it('acknowledges the save in the title — it succeeded', () => {
const sink = makeSink();

emitSaveAdvisories(event(), t, sink);

const [title] = sink.warning.mock.calls[0]!;
expect(title).toMatch(/^Saved/);
expect(title).toContain('1');
});

it('lists rule, message and hint for each finding', () => {
const sink = makeSink();

emitSaveAdvisories(event(), t, sink);

const description = sink.warning.mock.calls[0]![1]!.description!;
expect(description).toContain('flow/delete-without-filter');
expect(description).toContain(FINDING.message);
expect(description).toContain(FINDING.hint);
});

it('renders `where` as secondary context and omits the machine `path`', () => {
const sink = makeSink();

emitSaveAdvisories(event(), t, sink);

const description = sink.warning.mock.calls[0]![1]!.description!;
expect(description).toContain('flow "nightly_purge" · node "purge old rows"');
expect(description).not.toContain('flows[0].nodes[2].config.filters');
});

it('renders server prose verbatim — message and hint are not translated', () => {
const sink = makeSink();
// A `t` that mangles everything it is given. The finding text must survive
// it untouched, because it is server data and not an i18n key.
const shoutingT: TranslateFn = () => 'TRANSLATED-TITLE';

emitSaveAdvisories(event(), shoutingT, sink);

const [title, opts] = sink.warning.mock.calls[0]!;
expect(title).toBe('TRANSLATED-TITLE');
expect(opts!.description).toContain(FINDING.message);
expect(opts!.description).toContain(FINDING.hint);
});

it('lists every finding when the gate returned several', () => {
const sink = makeSink();
const second = { ...FINDING, rule: 'flow/unreachable-node', message: 'node 4 is unreachable' };

emitSaveAdvisories(event({ advisories: [FINDING, second] }), t, sink);

const [title, opts] = sink.warning.mock.calls[0]!;
expect(title).toContain('2');
expect(opts!.description).toContain('flow/delete-without-filter');
expect(opts!.description).toContain('flow/unreachable-node');
expect(opts!.description!.split('\n')).toHaveLength(2);
});

it('stays on screen long enough to be read', () => {
const sink = makeSink();

emitSaveAdvisories(event(), t, sink);

expect(sink.warning.mock.calls[0]![1]!.duration).toBeGreaterThanOrEqual(10_000);
});

it('says nothing when there is nothing to say — a clean save renders no new UI', () => {
const sink = makeSink();

emitSaveAdvisories(event({ advisories: [] }), t, sink);

expect(sink.warning).not.toHaveBeenCalled();
expect(sink.error).not.toHaveBeenCalled();
expect(sink.success).not.toHaveBeenCalled();
});

it('tolerates a finding with no `where`', () => {
const sink = makeSink();
const bare = { ...FINDING, where: '' };

emitSaveAdvisories(event({ advisories: [bare] }), t, sink);

const description = sink.warning.mock.calls[0]![1]!.description!;
expect(description).toContain('[flow/delete-without-filter]');
expect(description).toContain(FINDING.message);
});
});
105 changes: 105 additions & 0 deletions packages/app-shell/src/providers/saveAdvisoryToast.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/**
* 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.
*/

/**
* Turning the runtime authoring gate's advisory findings (objectui#4133,
* backend objectstack#7435) into the message the author reads.
*
* Lives apart from the client factory that wires it — and, deliberately,
* imports NOTHING that renders — so the wording and the grouping can be
* exercised directly. Same split, and the same reason, as its sibling
* `writeWarningToast.ts`: the caller owns the sink, the test hands over its
* own, and no module mock is needed.
*
* ## Why the save is acknowledged, never failed
*
* These findings ride a **200**. The row persisted, the version bumped, and the
* author's change is live; the gate merely noticed something worth saying about
* it. So the affordance is the warning tier, and the title says "Saved" first —
* the mistake this module exists to avoid is a successful save that reads as a
* failure. `severity` on this channel is never `'error'` (an error-severity
* finding is a 422 and never reaches a 2xx body), which is why nothing here
* branches on it.
*
* ## Why the finding text is not translated
*
* `message` and `hint` are SERVER data — composed by the gate's rules, which
* name the offending node, object and field. They are rendered verbatim. Only
* the frame around them (the title, and the "rule — message" scaffolding) is
* i18n copy. Putting server prose through `t()` would need a key per rule and
* would go stale the moment a rule's wording changed upstream.
*
* @module providers/saveAdvisoryToast
*/

import type { MetadataSaveAdvisoryEvent } from '@object-ui/data-objectstack';

/** i18next's `t`, narrowed to what this module uses. */
export type TranslateFn = (key: string, options?: Record<string, unknown>) => string;

/**
* Where the message goes. Structurally satisfied by sonner's `toast`, which is
* what the console metadata client factory passes.
*
* Required rather than defaulted to `sonner` for the same reason the write
* warning sink is: a default would mean importing the toaster here, which is
* precisely the dependency that has to stay out of this module.
*/
export interface SaveAdvisorySink {
warning(title: string, options?: { description?: string; duration?: number }): void;
}

/**
* How long the advisory stays on screen. Longer than the 4s default because
* the body is multi-line prose the author has to actually read — the same
* 10s the pre-publish capability lint uses for its findings toast
* (`preview/usePublishAllDrafts.ts`), so the two advisory surfaces behave
* alike.
*/
const ADVISORY_TOAST_MS = 10_000;

/**
* Render one finding as a line: the rule id that produced it, its message, and
* its remedy. `where` is appended as secondary context when the gate supplied
* it — it names the flow/node the finding is about, which is what the author
* needs to go fix it. `path` is deliberately NOT rendered: it is a document
* pointer (`flows[0].nodes[2].config…`) meant for tooling, and next to a
* human-readable `where` it reads as noise.
*/
function formatFinding(f: MetadataSaveAdvisoryEvent['advisories'][number]): string {
const head = f.where ? `[${f.rule}] ${f.where}` : `[${f.rule}]`;
const hint = f.hint ? ` ${f.hint}` : '';
return `${head} — ${f.message}${hint}`;
}

/**
* Announce the gate's advisory findings for a save 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
* "0 findings".
*/
export function emitSaveAdvisories(
ev: MetadataSaveAdvisoryEvent,
t: TranslateFn,
sink: SaveAdvisorySink,
): void {
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)',
}),
{
description: ev.advisories.map(formatFinding).join('\n'),
duration: ADVISORY_TOAST_MS,
},
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
* never touches `credentials`, so a same-origin session cookie still flows.
*/

import { MetadataClient } from '@object-ui/data-objectstack';
import { MetadataClient, type MetadataSaveAdvisoryListener } from '@object-ui/data-objectstack';
import { createAuthenticatedFetch } from '@object-ui/auth';

/**
Expand Down Expand Up @@ -63,6 +63,15 @@ export interface ConsoleMetadataClientOptions {
previewDrafts?: boolean;
/** Scope reads/writes to a tenant environment (`withEnvironment`). */
environmentId?: string;
/**
* #4133 — sink for the runtime authoring gate's advisory findings on a
* SUCCESSFUL save (objectstack#7435). Passed straight through to
* {@link MetadataClient}; `useMetadataClient` supplies the console's toast
* renderer, which is what puts one wiring in front of every save call site
* in the app. Omitted for read-only clients (`MetadataProvider`), which
* never write and so can never receive one.
*/
onSaveAdvisory?: MetadataSaveAdvisoryListener;
}

/**
Expand All @@ -72,11 +81,12 @@ export interface ConsoleMetadataClientOptions {
export function createConsoleMetadataClient(
options: ConsoleMetadataClientOptions = {},
): MetadataClient {
const { previewDrafts = false, environmentId } = options;
const { previewDrafts = false, environmentId, onSaveAdvisory } = options;
const client = new MetadataClient({
baseUrl: resolveMetadataBaseUrl(),
previewDrafts,
fetch: consoleApiFetch,
...(onSaveAdvisory ? { onSaveAdvisory } : {}),
});
return environmentId ? client.withEnvironment(environmentId) : client;
}
34 changes: 30 additions & 4 deletions packages/app-shell/src/views/metadata-admin/useMetadata.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,13 @@
* consumers.
*/

import { useEffect, useMemo, useState } from 'react';
import { type MetadataClient, type MetadataDiagnosticsSummary, type MetadataDiagnosticsEntry } from '@object-ui/data-objectstack';
import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { type MetadataClient, type MetadataDiagnosticsSummary, type MetadataDiagnosticsEntry, type MetadataSaveAdvisoryEvent } from '@object-ui/data-objectstack';
import { useObjectTranslation } from '@object-ui/i18n';
import { toast } from 'sonner';
import { usePreviewDrafts } from '../../preview/PreviewModeContext';
import { createConsoleMetadataClient } from './metadataClientFactory';
import { emitSaveAdvisories, type TranslateFn } from '../../providers/saveAdvisoryToast';

/**
* A declarative **type-level** action surfaced on a metadata type by the
Expand Down Expand Up @@ -105,9 +108,32 @@ export function useMetadataClient(environmentId?: string): MetadataClient {
// ADR-0037: inside a draft-preview tree (?preview=draft), reads overlay
// pending drafts on the active registry. Writes are unaffected.
const previewDrafts = usePreviewDrafts();

// #4133 — the runtime authoring gate's advisory findings, rendered ONCE for
// every save call site in the app.
//
// This hook is why the wiring belongs here rather than at the ~20 places
// that call `client.save(...)`: every app-shell write path takes its client
// from `useMetadataClient` (ResourceEditPage, StudioDesignSurface,
// EmbeddedItemEditor, DatasourceResourcePage, ObjectHooksPanel, …), so one
// sink here covers all of them and any future one for free. The factory
// below stays render-free; this is the layer that has React context, which
// is what the translation needs.
//
// `t` is read through a ref so a locale change does not re-mint the client
// (which would remount every consumer) while the toast still renders in the
// CURRENT language — the same shape `AdapterProvider` uses for its sibling
// write-warning channel.
const { t } = useObjectTranslation();
const tRef = useRef(t);
tRef.current = t;
const onSaveAdvisory = useCallback((ev: MetadataSaveAdvisoryEvent) => {
emitSaveAdvisories(ev, tRef.current as TranslateFn, toast);
}, []);

return useMemo(
() => createConsoleMetadataClient({ previewDrafts, environmentId }),
[environmentId, previewDrafts],
() => createConsoleMetadataClient({ previewDrafts, environmentId, onSaveAdvisory }),
[environmentId, previewDrafts, onSaveAdvisory],
);
}

Expand Down
5 changes: 4 additions & 1 deletion packages/data-objectstack/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3906,8 +3906,11 @@ export type { IntegrationConfig, IntegrationTrigger, IntegrationProvider, SlackI
// Used by plugin-designer to back the Setup-app Object Manager and Field
// Designer surfaces; kept separate from ObjectStackAdapter so callers
// can use it without the full data-source surface.
export { MetadataClient } from './metadata-client';
export { MetadataClient, readSaveAdvisories } from './metadata-client';
export type {
RuntimeAuthoringIssue,
MetadataSaveAdvisoryEvent,
MetadataSaveAdvisoryListener,
MetadataClientConfig,
MetadataListOptions,
MetadataDraftHeader,
Expand Down
Loading
Loading