diff --git a/.changeset/26707-draft-diagnostics-publish-verdict.md b/.changeset/26707-draft-diagnostics-publish-verdict.md new file mode 100644 index 00000000000..95401efc4bf --- /dev/null +++ b/.changeset/26707-draft-diagnostics-publish-verdict.md @@ -0,0 +1,15 @@ +--- +'@objectstack/metadata-protocol': patch +--- + +fix(metadata-protocol): a draft's `_diagnostics` reports what its publish will refuse (#26707) + +Clause-②: no (the draft verdict reports more; no accepted value is refused) +Metadata-protocol: none + +A draft the runtime authoring gate refuses at publish (a flow edge whose condition does not parse, an API-triggered flow with no secret, an RLS predicate over a field the database cannot filter, a summary field with no operations, a dashboard global filter on an unknown field, a dataset `include` that names an unknown relationship) used to read `_diagnostics.valid: true` on the draft read, so the editors and the Problems tab called it valid and the refusal arrived only at Publish, where one such draft rolls back the whole package release. + +- **One rule set.** The draft read (`GET /api/v1/meta/:type/:name?state=draft`, and the `previewDrafts` read of a pending draft) now runs the same runtime authoring gate the publish runs, on the stored draft body, with the same package scope, the same pending in-package drafts (so a reference to a sibling draft the same Publish-all lands is not an error) and the same restored credential paths. Each refusal is added to `_diagnostics.errors` with the gate's rule id as `code` and the publish's own `message`, and `valid` becomes `false`. ⛔ No rule is restated. +- **The draft still saves.** No draft save is refused; publish refuses exactly what it refused before; package publish keeps its atomic rollback. +- **Paths.** A gate path inside the item (`objects.invoice.fields.total.summaryOperations`) is served item-relative (`fields.total.summaryOperations`); a gate path that names a place rather than a key (a flow edge) is served as the gate wrote it. +- **Not covered.** The save-door refusals (they refuse the save itself, so no stored draft carries them), the plugin authoring gates and the package build probes that `publish-drafts` runs, and the list reads and the diagnostics sweep, which keep their read-time verdicts. diff --git a/packages/metadata-protocol/src/metadata-diagnostics.ts b/packages/metadata-protocol/src/metadata-diagnostics.ts index d14d9b98d4c..7ef661e62d9 100644 --- a/packages/metadata-protocol/src/metadata-diagnostics.ts +++ b/packages/metadata-protocol/src/metadata-diagnostics.ts @@ -37,6 +37,11 @@ import { zodIssuesToMetadataIssues } from './protocol.js'; // landed by #8300). Composed into `decorateMetadataItem` below — see the // ordering note there for why it is not applied per read exit. import { redactMetadataItem } from './metadata-redaction.js'; +// [#26707] The snapshot key a write type's findings are spelled under — the +// runtime gate's own answer, so the publish verdict below is re-spelled +// against the same prefix the gate wrote it with. +import { stackKeyForType } from '@objectstack/lint/runtime'; +import type { RuntimeAuthoringIssue } from '@objectstack/spec/api'; /** * Re-export the canonical validation-result type so callers in this @@ -316,6 +321,64 @@ export function decorateMetadataItems(type: string, items: T[]): T[] { return items.map((item) => decorateMetadataItem(type, item)); } +// --------------------------------------------------------------------------- +// [#26707] The publish verdict on a draft's `_diagnostics` +// --------------------------------------------------------------------------- + +/** + * A runtime-gate finding's path, re-spelled relative to the item it was found + * on: the gate spells it inside its private per-write snapshot + * (`flows[0].edges[1].condition`, `objects.invoice.fields.total`), and + * `_diagnostics` paths are dotted and item-relative (`edges.1.condition`), the + * spelling Studio's inline errors and canvas badges resolve. A path that does + * not start at the written item (a finding located on a sibling, or a `where`- + * style locator) is kept as the gate wrote it. + */ +function itemRelativeIssuePath(type: string, name: string, path: string): string { + const stackKey = stackKeyForType(PLURAL_TO_SINGULAR[type] ?? type); + if (!stackKey) return path; + let rest: string | undefined; + for (const prefix of [`${stackKey}[0]`, `${stackKey}.${name}`]) { + if (path === prefix || path.startsWith(`${prefix}.`) || path.startsWith(`${prefix}[`)) { + rest = path.slice(prefix.length); + break; + } + } + if (rest === undefined) return path; + return rest.replace(/\[(\d+)\]/g, '.$1').replace(/^\./, ''); +} + +/** + * Fold what publishing a draft would refuse into the draft's `_diagnostics`. + * + * `issues` are the runtime authoring gate's refusal for the draft body — + * `ObjectStackProtocolImplementation#draftPublishVerdict`, the judgement the + * publish door enforces — and each becomes one `errors[]` entry carrying the + * publish's own `message`, with its `rule` id as the entry's `code`. Any of + * them makes the verdict `valid: false`: the draft is still stored, but it does + * not say it is valid while the publish says it is not. No issues: the schema + * verdict is returned as it was. A type with no schema verdict (`undefined`) + * gets one only when there is something to report. + */ +export function withPublishVerdict( + type: string, + name: string, + diagnostics: MetadataDiagnostics | undefined, + issues: readonly RuntimeAuthoringIssue[], +): MetadataDiagnostics | undefined { + if (issues.length === 0) return diagnostics; + const refusals = issues.map((issue) => ({ + path: itemRelativeIssuePath(type, name, issue.path), + message: issue.message, + code: issue.rule, + })); + return { + ...diagnostics, + valid: false, + errors: [...(diagnostics?.errors ?? []), ...refusals], + }; +} + // --------------------------------------------------------------------------- // ADR-0047 — reference-integrity diagnostics for list views // --------------------------------------------------------------------------- diff --git a/packages/metadata-protocol/src/protocol-authoring-gate.ts b/packages/metadata-protocol/src/protocol-authoring-gate.ts index 40f0c66f37e..c84d402af97 100644 --- a/packages/metadata-protocol/src/protocol-authoring-gate.ts +++ b/packages/metadata-protocol/src/protocol-authoring-gate.ts @@ -49,7 +49,7 @@ import type { ObjectStackProtocolImplementation } from './protocol.js'; import { permissionSetOwnerBindingRefusal, runtimeHookWithoutBodyRefusal } from './protocol.js'; import { evaluateRuntimeAuthoringGate } from './runtime-authoring-gate.js'; -import type { RuntimeAuthoringIssue, RuntimePendingDeclarations } from './runtime-authoring-gate.js'; +import type { RuntimeAuthoringIssue, RuntimeAuthoringVerdict, RuntimePendingDeclarations } from './runtime-authoring-gate.js'; import { savedItemNameRefusal } from '@objectstack/metadata/view-container-name'; import { PLURAL_TO_SINGULAR, SINGULAR_TO_PLURAL } from '@objectstack/spec/shared'; import type { IObjectQLEngine } from '@objectstack/spec/contracts'; @@ -67,11 +67,13 @@ export interface AuthoringGateHost { assertRuntimeAuthoringRules: ObjectStackProtocolImplementation['assertRuntimeAuthoringRules']; readonly authoringChannel: ObjectStackProtocolImplementation['authoringChannel']; readonly authoringGates: ObjectStackProtocolImplementation['authoringGates']; + collectBatchPendingDeclarations: ObjectStackProtocolImplementation['collectBatchPendingDeclarations']; credentialChannelFor: ObjectStackProtocolImplementation['credentialChannelFor']; defaultRecordPageSaveRefusal: ObjectStackProtocolImplementation['defaultRecordPageSaveRefusal']; readonly engine: ObjectStackProtocolImplementation['engine']; externalBindingSaveRefusal: ObjectStackProtocolImplementation['externalBindingSaveRefusal']; foldStoredCollection: ObjectStackProtocolImplementation['foldStoredCollection']; + getOverlayRepo: ObjectStackProtocolImplementation['getOverlayRepo']; hierarchyResolverMounted: ObjectStackProtocolImplementation['hierarchyResolverMounted']; isArtifactBacked: ObjectStackProtocolImplementation['isArtifactBacked']; orgWallEnforced: ObjectStackProtocolImplementation['orgWallEnforced']; @@ -167,9 +169,33 @@ export async function runAuthoringGate( }); } +/** + * The members {@link judgeRuntimeAuthoringRules} reads: the host facts and + * collections every judgement of the shared rules is made against. + */ +type RuntimeRulesHost = Pick; + /** The body of `ObjectStackProtocolImplementation#assertRuntimeAuthoringRules`; the method's docblock there is its contract. */ export async function assertRuntimeAuthoringRules( - this: Pick, + this: RuntimeRulesHost, + evt: Parameters[0], +): Promise { + const verdict = await judgeRuntimeAuthoringRules.call(this, evt); + if (verdict.error) throw verdict.error; + return verdict.advisories; +} + +/** + * [#26707] The judgement {@link assertRuntimeAuthoringRules} enforces, handed + * back instead of thrown: the gate's {@link RuntimeAuthoringVerdict}, its + * refusal (`error`) beside its advisories. The assert above is this function + * plus the throw, so a caller that only needs to KNOW the verdict — the draft + * read's publish verdict ({@link draftPublishVerdict}) — reads the same + * collections, the same host facts and the same early returns as the publish + * that enforces it, and there is no second account of either. + */ +export async function judgeRuntimeAuthoringRules( + this: RuntimeRulesHost, evt: { type: string; name: string; state: 'draft' | 'active'; body: unknown; source?: string; /** @@ -211,7 +237,8 @@ export async function assertRuntimeAuthoringRules( * round trip for a secret it still holds. */ restoredCredentialPaths?: () => Promise; -}): Promise { +}): Promise { + const clean: RuntimeAuthoringVerdict = { error: null, advisories: [] }; // [#6710] The ADR-0005 carve-out, now DECLARED instead of inferred. // // This line used to read `if (this.environmentId === undefined) @@ -250,21 +277,24 @@ export async function assertRuntimeAuthoringRules( // #6710 re-keyed this activation and left that one on the retired // proxy, which cost the ADR-0090 D11 object posture gate every // host-config deployment until #7674 finished the move. - if (this.authoringChannel === 'package-author') return []; + if (this.authoringChannel === 'package-author') return clean; if (evt.state !== 'active') { // [#26188] D1: no rule gates a draft. The one finding a draft does hear // is a hierarchy scope this runtime cannot enforce, as a warning on the // 2xx; it needs no collection read, so the draft keeps paying none. // A `migrate-stored` pass heals storage and hears nothing (see below). - if (evt.source === 'migrate-stored') return []; + if (evt.source === 'migrate-stored') return clean; const hierarchyResolverMounted = this.hierarchyResolverMounted(); - return evaluateRuntimeAuthoringGate({ - type: PLURAL_TO_SINGULAR[evt.type] ?? evt.type, - name: evt.name, - state: evt.state, - body: evt.body, - ...(hierarchyResolverMounted !== undefined ? { hierarchyResolverMounted } : {}), - }).advisories; + return { + error: null, + advisories: evaluateRuntimeAuthoringGate({ + type: PLURAL_TO_SINGULAR[evt.type] ?? evt.type, + name: evt.name, + state: evt.state, + body: evt.body, + ...(hierarchyResolverMounted !== undefined ? { hierarchyResolverMounted } : {}), + }).advisories, + }; } // `os migrate meta --stored --apply` rewrites rows that ALREADY EXIST // into the current dialect. It is not an author publishing anything — @@ -280,7 +310,7 @@ export async function assertRuntimeAuthoringRules( // no caller can spell its way past the gate. `duplicatePackage` is // deliberately NOT here — it mints brand-new rows under new names, and // a copy of a broken flow is a new broken flow. - if (evt.source === 'migrate-stored') return []; + if (evt.source === 'migrate-stored') return clean; const singular = PLURAL_TO_SINGULAR[evt.type] ?? evt.type; // Resolution context. Best-effort: a host without a registry (a @@ -420,6 +450,39 @@ export async function assertRuntimeAuthoringRules( // [#26188] With it the gate refuses a hierarchy scope no resolver enforces. ...(hierarchyResolverMounted !== undefined ? { hierarchyResolverMounted } : {}), }); - if (verdict.error) throw verdict.error; - return verdict.advisories; + return verdict; +} + +/** The body of `ObjectStackProtocolImplementation#draftPublishVerdict`; the method's docblock there is its contract. */ +export async function draftPublishVerdict( + this: RuntimeRulesHost & Pick, + draft: { type: string; name: string; body: unknown; packageId: string | null }, +): Promise { + const singular = PLURAL_TO_SINGULAR[draft.type] ?? draft.type; + // The closure "Publish all" judges this draft against: its package's other + // drafts, collected by the batch door's own collector from the batch door's + // own listing. A package-unbound draft has no batch; its publish is the + // per-item door's, which states none. + let pending: RuntimePendingDeclarations | undefined; + if (draft.packageId) { + const repo = this.getOverlayRepo(); + if (typeof repo.listDrafts === 'function') { + pending = await this.collectBatchPendingDeclarations(await repo.listDrafts({ packageId: draft.packageId })); + } + } + // The promotion's restored-credential read: the stored draft holds no + // credential the write-only channel holds, so the gate is told where one is. + const channel = this.credentialChannelFor(singular); + const verdict = await judgeRuntimeAuthoringRules.call(this, { + type: singular, + name: draft.name, + state: 'active', + body: draft.body, + packageId: draft.packageId, + ...(pending !== undefined ? { pending } : {}), + ...(channel + ? { restoredCredentialPaths: () => channel.heldPaths({ name: draft.name, state: 'draft', item: draft.body }) } + : {}), + }); + return (verdict.error as { issues?: RuntimeAuthoringIssue[] } | null)?.issues ?? []; } diff --git a/packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts b/packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts new file mode 100644 index 00000000000..8edf32595bd --- /dev/null +++ b/packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts @@ -0,0 +1,442 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #26707 — a draft's `_diagnostics` says what its publish will say. + * + * ## The defect, measured on `origin/main` before this file was written + * + * Six author-time rules ran only at publish. Each of the bodies below saved as + * a draft (200), its draft read answered `_diagnostics: { valid: true }`, and + * the publish refused it 422 `INVALID_METADATA` with the rule id in + * `issues[]`. In a package publish the refusal rolled every other draft of + * the package back with it, so an author who trusted "valid" lost unrelated + * work far from the edit that caused it. + * + * ## What is pinned + * + * - The draft still SAVES (#4463 D1 is unchanged): every fixture is stored. + * - Its read reports `valid: false` with the publish's own rule id (as the + * entry's `code`) and the publish's own message — compared against the + * publish run in the same test, never against a restatement of it. + * - The publish still refuses exactly as before. + * - A clean draft stays `valid: true`, and a draft whose reference resolves + * only inside its own package (the closure "Publish all" judges) is not + * reported: the verdict is the package publish's, not a stricter one. + * + * Harness: the package-aware multi-table stub engine of + * `protocol-publish-drafts-closure.test.ts` (kept local, the established + * shape). Nothing on the save, read or publish path is stubbed. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate } from '@objectstack/metadata-core'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +/** The LIVE object universe: a deal with a formula field, and its account. */ +const liveDealObject = { + name: 'deal', + label: 'Deal', + sharingModel: 'private', + fields: { + name: { type: 'text', label: 'Name' }, + amount: { type: 'number', label: 'Amount' }, + deal_score: { + type: 'formula', + label: 'Score', + expression: { dialect: 'cel', source: 'record.amount * 2' }, + returnType: 'number', + }, + }, +}; + +/** The LIVE dataset a fixture dashboard binds. */ +const liveDealDataset = { + name: 'deals_live', + label: 'Deals', + object: 'deal', + dimensions: [{ name: 'deal_name', field: 'name' }], + measures: [{ name: 'deal_count', aggregate: 'count' }], +}; + +interface Row { + id: string; + type: string; + name: string; + organization_id: string | null; + package_id: string | null; + state: string; + metadata: string; + checksum?: string; + version?: number; + updated_at?: string; + created_at?: string; +} + +interface HistoryRow { + id: string; + event_seq: number; + name: string; + type: string; + version: number; + operation_type: string; + metadata: string | null; + checksum: string | null; + previous_checksum: string | null; + change_note?: string | null; + source?: string | null; + organization_id: string | null; + recorded_by?: string | null; + recorded_at: string; +} + +// Overlay rows are keyed by (type, name, org, state, package_id) — the ADR-0048 key. +function keyOf(w: Record) { + return `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`; +} + +function matchesMetadataWhere(r: Row, where: Record): boolean { + for (const [k, v] of Object.entries(where)) { + if (k === '$or') { + const clauses = v as Array>; + if (!clauses.some((c) => matchesMetadataWhere(r, c))) return false; + continue; + } + if (v === undefined) continue; + if ((r as any)[k] !== v) return false; + } + return true; +} + +function makeStubEngine() { + const rows = new Map(); + const historyRows: HistoryRow[] = []; + let nextId = 0; + + const findRow = (w: Record): { key: string; row: Row } | null => { + if (w.id !== undefined) { + for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r }; + return null; + } + if (w.package_id !== undefined) { + const k = keyOf(w); + const r = rows.get(k); + return r ? { key: k, row: r } : null; + } + for (const [k, r] of rows) if (matchesMetadataWhere(r, w)) return { key: k, row: r }; + return null; + }; + + const matchesHistory = (h: HistoryRow, w: Record): boolean => { + if (w.organization_id !== undefined && h.organization_id !== w.organization_id) return false; + if (w.type !== undefined && h.type !== w.type) return false; + if (w.name !== undefined && h.name !== w.name) return false; + if (w.version !== undefined && h.version !== w.version) return false; + if (w.operation_type !== undefined && h.operation_type !== w.operation_type) return false; + return true; + }; + + const engine: any = { + async findOne(table: string, opts: { where: Record }) { + assertEngineFindOnePredicate(table, opts); + if (table === 'sys_metadata_history') { + return historyRows.find((h) => matchesHistory(h, opts.where)) ?? null; + } + return findRow(opts.where)?.row ?? null; + }, + async find(table: string, opts: { where: Record; limit?: number }) { + const matched = table === 'sys_metadata_history' + ? historyRows.filter((h) => matchesHistory(h, opts.where)) + : Array.from(rows.values()).filter((r) => matchesMetadataWhere(r, opts.where)); + return typeof opts?.limit === 'number' ? matched.slice(0, opts.limit) : matched; + }, + async insert(table: string, data: Record) { + if (table === 'sys_metadata_audit') return { id: 'audit_skip' }; + if (table === 'sys_metadata_history') { + nextId += 1; + const h: HistoryRow = { id: `h_${nextId}`, ...(data as any) }; + historyRows.push(h); + return { id: h.id }; + } + nextId += 1; + const row = { id: `r_${nextId}`, ...(data as any) } as Row; + rows.set(keyOf(data), row); + return { id: row.id }; + }, + async update(_t: string, data: Record, opts: { where: Record }) { + assertEngineUpdateDispatch(data, opts); + const found = findRow(opts.where); + if (!found) return { id: null }; + const merged = { ...found.row, ...(data as any) }; + rows.delete(found.key); + rows.set(keyOf(merged), merged); + return { id: found.row.id }; + }, + async delete(_t: string, opts: { where: Record }) { + assertEngineDeleteDispatch(opts); + const found = findRow(opts.where); + if (!found) return { deleted: 0 }; + rows.delete(found.key); + return { deleted: 1 }; + }, + async transaction(cb: (ctx: any, info: { owned: boolean }) => Promise): Promise { + return cb(undefined, { owned: true }); + }, + // The engine's judge-only filter admission (#20158), as the live engine + // answers it for a read scope on a formula field: refused, because a + // virtual field has no column a row filter can read. The engine's real + // verdicts are pinned in `packages/cli/test/rls-policy-authoring-admission.test.ts`. + judgeFilter: (_object: string, where: unknown) => + JSON.stringify(where).includes('deal_score') + ? { ok: false, code: 'FILTER_FIELD_NOT_STORED', status: 400, message: 'Field "deal_score" is a formula field and cannot be filtered.' } + : { ok: true }, + registry: { + registerItem: () => {}, + registerObject: () => {}, + // The LIVE universe: the objects and the one dataset the fixtures + // resolve against. + listItems: (type: string) => { + if (type === 'object') return [liveDealObject]; + if (type === 'dataset') return [liveDealDataset]; + return []; + }, + // The base the drafts are bound to is INSTALLED — a flow may be + // saved only into a package the registry holds (#20863) — and its + // manifest declares no namespace → the ADR-0028 prefix pre-flight is + // skipped (legacy-grandfathered path); it declares no dependencies + // and no live object carries a package stamp, so + // `resolveWritePackageScope`'s closure removes nothing. + getPackage: (id: string) => (id === PKG ? { manifest: { id: PKG } } : undefined), + }, + }; + return { engine, rows, historyRows }; +} +const PKG = 'app.sales'; + +// ───────────────────────────────────────────────────────────────────────────── +// The card's six shapes — every one Zod-valid, so a draft of it saves, and +// each one refused at publish by the rule named beside it. +// ───────────────────────────────────────────────────────────────────────────── + +const CASES: Array<{ rule: string; type: string; item: { name: string } & Record }> = [ + { + // Reproduction 1: Start → Decision → End, a conditional edge whose CEL does not parse. + rule: 'expression-invalid', + type: 'flow', + item: { + name: 'edge_cel', + label: 'Edge CEL', + type: 'autolaunched', + nodes: [ + { id: 'start', type: 'start', label: 'Start' }, + { id: 'decide', type: 'decision', label: 'Decide' }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [ + { id: 'e1', source: 'start', target: 'decide' }, + { id: 'e2', source: 'decide', target: 'end', type: 'conditional', condition: 'record.name ==' }, + ], + }, + }, + { + // Reproduction 2: a Webhook / API trigger with no secret. + rule: 'flow-api-trigger-secret-missing', + type: 'flow', + item: { + name: 'inbound_hook', + label: 'Inbound Hook', + type: 'api', + nodes: [ + { id: 'start', type: 'start', label: 'Start', config: { triggerType: 'api' } }, + { id: 'end', type: 'end', label: 'End' }, + ], + edges: [{ id: 'e1', source: 'start', target: 'end' }], + }, + }, + { + // Reproduction 3: an RLS read scope on a virtual (formula) field. + rule: 'rls-predicate-unenforceable', + type: 'permission', + item: { + name: 'sales_team', + label: 'Sales Team', + objects: { deal: { allowRead: true } }, + rowLevelSecurity: [{ name: 'hot_deals', object: 'deal', operation: 'select', using: 'record.deal_score > 5' }], + }, + }, + { + // Reproduction 4: a summary field with no summaryOperations. + rule: 'field/summary-without-operations', + type: 'object', + item: { + name: 'invoice', + label: 'Invoice', + sharingModel: 'private', + fields: { + name: { type: 'text', label: 'Name' }, + total: { type: 'summary', label: 'Total' }, + }, + }, + }, + { + // Reproduction 5: a dashboard global filter on a field the bound + // widget's object does not have. + rule: 'dashboard-filter-field-unknown', + type: 'dashboard', + item: { + name: 'deal_board', + label: 'Deal Board', + globalFilters: [{ field: 'region', type: 'select' }], + widgets: [{ id: 'kpi_deals', type: 'metric', title: 'Deals', dataset: 'deals_live', values: ['deal_count'] }], + }, + }, + { + // Reproduction 6: a dataset whose include names no relationship. + rule: 'dataset-include-unknown', + type: 'dataset', + item: { + name: 'deal_rollup', + label: 'Deal Rollup', + object: 'deal', + include: ['no_such_relation'], + dimensions: [{ name: 'deal_name', field: 'name' }], + measures: [{ name: 'deal_count', aggregate: 'count' }], + }, + }, +]; + +/** Stage one env-wide, package-bound draft (Studio's autosave shape). */ +async function stageDraft(protocol: ObjectStackProtocolImplementation, type: string, item: { name: string }) { + return (protocol as any).saveMetaItem({ type, name: item.name, item, packageId: PKG, mode: 'draft' }); +} + +/** The draft read Studio's editor makes, and its `_diagnostics`. */ +async function draftDiagnostics(protocol: ObjectStackProtocolImplementation, type: string, name: string) { + const read: any = await protocol.getMetaItem({ type, name, state: 'draft', packageId: PKG } as any); + return read.item._diagnostics as { valid: boolean; errors?: Array<{ path: string; message: string; code?: string }> }; +} + +const pairs = (list: Array<{ rule?: string; code?: string; message: string }>) => + list.map((e) => [e.rule ?? e.code, e.message]).sort(); + +describe('a draft reads the verdict its publish gives (#26707)', () => { + let warn: ReturnType; + beforeEach(() => { + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + delete process.env.OS_ALLOW_UNLINTED_METADATA_WRITES; + }); + afterEach(() => { + warn.mockRestore(); + delete process.env.OS_ALLOW_UNLINTED_METADATA_WRITES; + }); + + for (const c of CASES) { + it(`${c.type} — [${c.rule}]: the draft saves, reads valid:false, and publish refuses with the same words`, async () => { + const { engine, rows } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + + // The draft still saves: no draft is refused by this card. + const saved = await stageDraft(protocol, c.type, c.item); + expect(saved).toMatchObject({ success: true, state: 'draft' }); + expect([...rows.values()].filter((r) => r.name === c.item.name).map((r) => r.state)).toEqual(['draft']); + + const diagnostics = await draftDiagnostics(protocol, c.type, c.item.name); + + // The publish refuses it, as it did before this card: one 422 on the + // per-item door… + const refusal: any = await protocol + .publishMetaItem({ type: c.type, name: c.item.name, packageId: PKG } as any) + .then(() => null, (e: unknown) => e); + expect({ code: refusal?.code, status: refusal?.status }).toEqual({ code: 'INVALID_METADATA', status: 422 }); + expect(refusal.issues.map((i: any) => i.rule)).toContain(c.rule); + + // …and the draft's verdict is that refusal: valid:false, every + // refused rule as an entry's `code`, with the publish's message. + expect(diagnostics.valid).toBe(false); + expect(pairs(diagnostics.errors ?? [])).toEqual(pairs(refusal.issues)); + }); + } + + it('the package publish refuses each of them and rolls the batch back, as before', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + for (const c of CASES) await stageDraft(protocol, c.type, c.item); + + const res: any = await protocol.publishPackageDrafts({ packageId: PKG }); + + expect(res.success).toBe(false); + expect(res.publishedCount).toBe(0); + // The batch stops at its first refusal; that draft's own read named it. + const first = res.failed[0]; + const diagnostics = await draftDiagnostics(protocol, first.type, first.name); + expect(diagnostics.valid).toBe(false); + }); + + it('a clean draft still reads valid:true, with no entry added', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + const flow = structuredClone(CASES[0]!.item) as any; + flow.edges[1].condition = "record.name == 'x'"; + await stageDraft(protocol, 'flow', flow); + + expect(await draftDiagnostics(protocol, 'flow', flow.name)).toEqual({ valid: true }); + const published: any = await protocol.publishPackageDrafts({ packageId: PKG }); + expect(published).toMatchObject({ success: true, failedCount: 0 }); + }); + + it('a reference that resolves inside its own package is judged as "Publish all" judges it — not reported', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + // A dashboard drafted beside the dataset it binds: neither is live. + await stageDraft(protocol, 'dataset', { ...liveDealDataset, name: 'deals_pending' }); + await stageDraft(protocol, 'dashboard', { + name: 'pending_board', + label: 'Pending Board', + widgets: [{ id: 'kpi', type: 'metric', title: 'Deals', dataset: 'deals_pending', values: ['deal_count'] }], + } as any); + + expect(await draftDiagnostics(protocol, 'dashboard', 'pending_board')).toEqual({ valid: true }); + const res: any = await protocol.publishPackageDrafts({ packageId: PKG }); + expect(res).toMatchObject({ success: true, failedCount: 0 }); + }); + + it('the same dashboard with its dataset nowhere is reported on its draft', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await stageDraft(protocol, 'dashboard', { + name: 'pending_board', + label: 'Pending Board', + widgets: [{ id: 'kpi', type: 'metric', title: 'Deals', dataset: 'deals_pending', values: ['deal_count'] }], + } as any); + + const diagnostics = await draftDiagnostics(protocol, 'dashboard', 'pending_board'); + expect(diagnostics.valid).toBe(false); + expect(diagnostics.errors?.map((e) => e.code)).toContain('widget-dataset-unknown'); + }); + + it('the draft-overlay preview read carries the same verdict', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + const c = CASES[3]!; + await stageDraft(protocol, c.type, c.item); + + const strict = await draftDiagnostics(protocol, c.type, c.item.name); + const preview: any = await protocol.getMetaItem({ type: c.type, name: c.item.name, previewDrafts: true, packageId: PKG } as any); + + expect(preview.item._draft).toBe(true); + expect(preview.item._diagnostics).toEqual(strict); + }); + + it('paths are item-relative where the finding is on the item itself', async () => { + const { engine } = makeStubEngine(); + const protocol = new ObjectStackProtocolImplementation(engine); + await stageDraft(protocol, 'object', CASES[3]!.item); + await stageDraft(protocol, 'flow', CASES[1]!.item); + + expect((await draftDiagnostics(protocol, 'object', 'invoice')).errors).toContainEqual( + expect.objectContaining({ path: 'fields.total.summaryOperations', code: 'field/summary-without-operations' }), + ); + expect((await draftDiagnostics(protocol, 'flow', 'inbound_hook')).errors).toContainEqual( + expect.objectContaining({ path: 'nodes.0.config.secret', code: 'flow-api-trigger-secret-missing' }), + ); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.item-execution-read.test.ts b/packages/metadata-protocol/src/protocol.item-execution-read.test.ts index 15e40f8e87f..9cd7eb9c5b5 100644 --- a/packages/metadata-protocol/src/protocol.item-execution-read.test.ts +++ b/packages/metadata-protocol/src/protocol.item-execution-read.test.ts @@ -13,6 +13,10 @@ // decoration (`_diagnostics` computed on the raw body, then the type's // credential redaction), so this one equality says "the served item minus // `_diagnostics` and redaction", for a redacting type as for a plain one. +// A PENDING DRAFT's served item also carries what its publish will refuse +// (#26707): `_diagnostics` is the decoration's verdict extended by the +// protocol's own `draftPublishVerdict` on the draft body +// (`withPublishVerdict`), the execution face carrying neither. // - THE ENVELOPE: every other key (`lock`, `editable`, `version`, `packageId`, // …) is equal, save `registration`, which the served face alone carries, // as the list read's served face alone carries `registrations`. @@ -30,7 +34,7 @@ import { describe, it, expect } from 'vitest'; import { assertEngineFindOnePredicate } from '@objectstack/metadata-core'; import { ObjectStackProtocolImplementation } from './protocol.js'; -import { decorateMetadataItem } from './metadata-diagnostics.js'; +import { decorateMetadataItem, withPublishVerdict } from './metadata-diagnostics.js'; import { metaOverlayCacheEntryCount } from './meta-overlay-cache.js'; type Row = Record; @@ -139,6 +143,14 @@ function harness(rows: Row[], opts: { disabledObjects?: string[]; automation?: b return { protocol, engine, reads, write }; } +/** + * [#26707] The publish verdict a pending draft's served read carries: the + * protocol's own judge, on the stored draft body. + */ +async function draftVerdictOf(h: ReturnType, type: string, body: Row): Promise { + return h.protocol.draftPublishVerdict({ type, name: body.name, body, packageId: null }); +} + /** Read one request on both faces. */ async function bothFaces(h: ReturnType, request: Row) { const served = await h.protocol.getMetaItem(structuredClone(request)); @@ -150,11 +162,16 @@ async function bothFaces(h: ReturnType, request: Row) { * THE parity: the served item is the execution item decorated, every other * envelope key is equal, and the served item really is decorated. */ -function expectParity(type: string, served: any, execution: any) { +function expectParity(type: string, served: any, execution: any, publishVerdict?: unknown[]) { expect(served.item, 'the served face found an item').toBeTruthy(); expect(served.item._diagnostics, 'the served item is decorated (anti-vacuity)').toBeDefined(); expect(Object.prototype.hasOwnProperty.call(execution.item, '_diagnostics')).toBe(false); - expect(served.item).toEqual(decorateMetadataItem(type, execution.item)); + const decorated = decorateMetadataItem(type, execution.item); + expect(served.item).toEqual( + publishVerdict === undefined + ? decorated + : { ...decorated, _diagnostics: withPublishVerdict(type, execution.item.name, decorated._diagnostics, publishVerdict as never) }, + ); const { item: _servedItem, registration: _servedOnly, ...servedEnvelope } = served; const { item: _executionItem, ...executionEnvelope } = execution; expect(executionEnvelope).toEqual(servedEnvelope); @@ -213,7 +230,10 @@ describe('[#24535] the by-name execution read is the served read minus the servi ]; const h = harness(rows); const { served, execution } = await bothFaces(h, { type: 'object', name: 'stored_widget', state: 'draft' }); - expectParity('object', served, execution); + const verdict = await draftVerdictOf(h, 'object', JSON.parse(rows[1].metadata as string)); + // The draft declares no sharingModel, which its publish refuses (#26707). + expect(verdict).toContainEqual(expect.objectContaining({ rule: 'security-owd-unset' })); + expectParity('object', served, execution, verdict); expect(execution.item.label).toBe('Draft'); }); @@ -223,7 +243,7 @@ describe('[#24535] the by-name execution read is the served read minus the servi objectRow('stored_widget', 'draft', { label: 'Draft' }), ]); const preview = await bothFaces(withDraft, { type: 'object', name: 'stored_widget', previewDrafts: true }); - expectParity('object', preview.served, preview.execution); + expectParity('object', preview.served, preview.execution, await draftVerdictOf(withDraft, 'object', { name: 'stored_widget', fields: {}, label: 'Draft' })); expect(preview.execution.item).toMatchObject({ label: 'Draft', _draft: true }); const withoutDraft = harness([objectRow('stored_widget', 'active', { label: 'Active' })]); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 4ccf8acedb4..e3ab9f0634a 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -217,6 +217,7 @@ import { decorateMetadataItem, decorateMetadataItems, type MetadataDiagnostics, + withPublishVerdict, } from './metadata-diagnostics.js'; // [#8154] The per-type read-path redaction seam. `decorateMetadataItem` already // composes `redactMetadataItem` (see its ordering note), which covers the @@ -360,6 +361,7 @@ import { // [#25467 cut 8] The runtime authoring gate and the restored-body doors. import { assertRuntimeAuthoringRules, + draftPublishVerdict, refuseRestoredBodyTheSaveDoorRefuses, restoredBodyDerivation, restoredBodyWriter, @@ -7177,6 +7179,52 @@ export class ObjectStackProtocolImplementation implements return assertRuntimeAuthoringRules.call(host, evt); } + /** + * [#26707] What publishing a stored draft would answer from the runtime + * authoring gate, without publishing it: the refusal's `issues` (rule id, + * path, message, exactly as the publish 422 carries them), or `[]` when + * the publish would pass the gate. + * + * The draft save stays ungated (#4463 D1): a draft may be half-finished and + * is stored as before. What changes is what its READ says about it. The + * draft's `_diagnostics` used to be the schema parse alone, so a draft the + * publish refuses (`expression-invalid` on a flow edge, a summary field + * with no operations, …) read `valid: true` in every editor until the + * author pressed Publish, and in a package publish it rolled back every + * other draft with it. + * + * One judgement, never a second copy of the rules: this reads + * `judgeRuntimeAuthoringRules`, the function the publish gate + * ({@link assertRuntimeAuthoringRules}) throws from, with the arguments the + * draft→active promotion states — the draft row's package (whose other + * drafts are the closure "Publish all" judges it against, read by the + * batch door's own collector) and the credential channel's held positions. + * Computed on every draft read rather than stored at save: the verdict + * depends on the live universe and the package's other drafts, which a + * stored verdict would not see change. + */ + private draftPublishVerdict(draft: { + type: string; name: string; body: unknown; packageId: string | null; + }): Promise { + const readAuthoringChannel = () => this.authoringChannel; + const readEngine = () => this.engine; + const host: ThisParameterType = { + get authoringChannel() { return readAuthoringChannel(); }, + get engine() { return readEngine(); }, + collectBatchPendingDeclarations: (...a) => this.collectBatchPendingDeclarations(...a), + credentialChannelFor: (...a) => this.credentialChannelFor(...a), + foldStoredCollection: (...a) => this.foldStoredCollection(...a), + getOverlayRepo: (...a) => this.getOverlayRepo(...a), + hierarchyResolverMounted: (...a) => this.hierarchyResolverMounted(...a), + orgWallEnforced: (...a) => this.orgWallEnforced(...a), + resolveSduiManifest: (...a) => this.resolveSduiManifest(...a), + resolveWritePackageScope: (...a) => this.resolveWritePackageScope(...a), + rlsMembershipResolverKeys: (...a) => this.rlsMembershipResolverKeys(...a), + tenancyPostureInForce: (...a) => this.tenancyPostureInForce(...a), + }; + return draftPublishVerdict.call(host, draft); + } + /** * [#15950] Fold the STORED half of a resolution universe onto the registry * half {@link assertRuntimeAuthoringRules} gathers. @@ -10744,6 +10792,25 @@ export class ObjectStackProtocolImplementation implements // Every read of `execution` below is one of those exits. const execution = opts.face === 'execution'; const serveItem = (resolved: T): T => (execution ? resolved : decorateMetadataItem(request.type, resolved)); + // [#26707] A served DRAFT is decorated as above and then told what its + // publish would refuse: the runtime gate's verdict on the stored draft + // body ({@link draftPublishVerdict}), folded into `_diagnostics` + // ({@link withPublishVerdict}). Judged on the row as stored — the body + // the promotion judges — never on the governed, converted or redacted + // copy this read serves. The execution face serves no `_diagnostics`. + const serveDraft = async (resolved: T, row: { metadata?: unknown; package_id?: string | null }): Promise => { + const served = serveItem(resolved); + if (execution || !served || typeof served !== 'object') return served; + const issues = await this.draftPublishVerdict({ + type: request.type, + name: request.name, + body: storedRowDocument(row), + packageId: row.package_id ?? null, + }); + const before = (served as { _diagnostics?: MetadataDiagnostics })._diagnostics; + const after = withPublishVerdict(request.type, request.name, before, issues); + return after === before ? served : { ...served, _diagnostics: after }; + }; // ── [#20946, #20761 ruling rule 1, ADR-0126 §2 / §3] A shipped FLOW name ── // // `flow` is Regime C: the packaged base is locked, "⛔ Never silent @@ -10891,7 +10958,7 @@ export class ObjectStackProtocolImplementation implements return { type: request.type, name: request.name, - item: serveItem(this.governServedObject(request.type, draftItem)), + item: await serveDraft(this.governServedObject(request.type, draftItem), draftRec), }; } } catch (error) { @@ -10917,6 +10984,8 @@ export class ObjectStackProtocolImplementation implements // [#24395] The served draft row's per-key provenance — read off the // row this read serves, so it can never describe another body. let draftChanges: ReturnType; + // [#26707] The draft row this read serves, for its publish verdict. + let servedDraftRow: { metadata?: unknown; package_id?: string | null } | undefined; try { let record = (await this.findServedOverlayRow({ type: request.type, @@ -10952,7 +11021,10 @@ export class ObjectStackProtocolImplementation implements if (recPkg && item && typeof item === 'object' && (item as any)._packageId === undefined) { (item as any)._packageId = recPkg; } - if (readState === 'draft') draftChanges = storedDraftChanges((record as { draft_changes?: unknown }).draft_changes); + if (readState === 'draft') { + draftChanges = storedDraftChanges((record as { draft_changes?: unknown }).draft_changes); + servedDraftRow = record as { metadata?: unknown; package_id?: string | null }; + } } } catch (error) { // [#5532] THE site this issue was raised on. Falling through to the @@ -10972,10 +11044,11 @@ export class ObjectStackProtocolImplementation implements // envelope shape. See ADR-0005 §draft-lifecycle. if (readState === 'draft') { if (item === undefined) throw noPendingDraftError(request.type, request.name); + const governedDraft = this.governServedObject(request.type, item); return { type: request.type, name: request.name, - item: serveItem(this.governServedObject(request.type, item)), + item: servedDraftRow ? await serveDraft(governedDraft, servedDraftRow) : serveItem(governedDraft), // [#22114] The draft row's version token — the one a draft save // (`?mode=draft`) at this address accepts as `If-Match`. // [#22877] The member's own row's, as a save of that member's diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index d13a5b82bc4..0bbbd36e8e3 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -746,6 +746,21 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/metadata-protocol/src/protocol.driver-text-disclosure.test.ts", "verb": "delete", diff --git a/ui/.changeset/26707-draft-server-verdict.md b/ui/.changeset/26707-draft-server-verdict.md new file mode 100644 index 00000000000..4f25fc758dd --- /dev/null +++ b/ui/.changeset/26707-draft-server-verdict.md @@ -0,0 +1,10 @@ +--- +'@object-ui/app-shell': patch +--- + +fix(app-shell): the edit page shows a pending draft's own server verdict (objectstack#26707) + +The edit page read `_diagnostics` from the layered read alone, which judges the published body, so a pending draft the publish refuses reached the canvas Problems panel and the diagnostics banner as valid. The draft read's `_diagnostics` now carries the publish's verdict on the draft, and the page takes it whenever it holds a draft: on load, after each autosave, after publish and after discard. Where a live client parse exists, the server errors the client parse does not raise are shown beside it, each once. + +Clause-②: no +Metadata-protocol: none diff --git a/ui/packages/app-shell/src/views/metadata-admin/ResourceEditPage.draftPublishVerdict-26707.test.tsx b/ui/packages/app-shell/src/views/metadata-admin/ResourceEditPage.draftPublishVerdict-26707.test.tsx new file mode 100644 index 00000000000..c55135a5905 --- /dev/null +++ b/ui/packages/app-shell/src/views/metadata-admin/ResourceEditPage.draftPublishVerdict-26707.test.tsx @@ -0,0 +1,195 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectstack#26707 — while a draft is pending, the editor shows the DRAFT's + * server verdict, which carries what its publish would refuse. + * + * ## The defect this file keeps fixed + * + * The edit page read `_diagnostics` from the layered read alone. That verdict + * judges the PUBLISHED body (`getMetaItemLayered` composes `effective` from the + * active rows and never reads a draft), so a pending draft the publish refuses + * — a flow edge whose CEL does not parse, a summary field with no operations — + * reached the canvas Problems panel and the diagnostics banner as "valid", and + * the refusal arrived only at Publish. Since objectstack#26707 the draft read's + * `_diagnostics` is the publish's own verdict on the draft; this page now takes + * it whenever it holds a draft. + * + * The server double below serves that verdict the way the real draft read + * does — computed from the stored draft body, with the rule id as `code` — and + * a layered verdict that says `valid: true`, as the published body is clean. + * Red on base: the canvas never receives the draft's error. + */ + +import '@testing-library/jest-dom/vitest'; +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, fireEvent, cleanup, waitFor } from '@testing-library/react'; +import { MemoryRouter } from 'react-router-dom'; + +type Row = Record; + +const REFUSAL = { + path: 'regions.0', + message: 'region "main" names a component this deployment does not have', + code: 'page-component-unknown', +}; + +const server = vi.hoisted(() => ({ + active: new Map>(), + drafts: new Map>(), + saves: 0, +})); + +const mockClient = vi.hoisted(() => { + const k = (type: string, name: string) => `${type}/${name}`; + /** The publish's verdict on a body: this double refuses a body labelled `broken`. */ + const verdictOf = (body: Record) => + body.label === 'broken' + ? { + valid: false, + errors: [{ + path: 'regions.0', + message: 'region "main" names a component this deployment does not have', + code: 'page-component-unknown', + }], + } + : { valid: true }; + return { + list: vi.fn(async () => []), + listDrafts: vi.fn(async () => []), + get: vi.fn(async () => null), + references: vi.fn(async () => []), + layered: vi.fn(async (type: string, name: string) => { + const eff = server.active.get(k(type, name)) ?? null; + return { + code: null, + overlay: eff, + overlayScope: eff ? 'env' : null, + effective: eff, + // The PUBLISHED body's verdict — clean. + ...(eff ? { _diagnostics: { valid: true } } : {}), + editable: true, + deletable: true, + resettable: false, + lock: 'none', + }; + }), + getDraft: vi.fn(async (type: string, name: string) => { + const row = server.drafts.get(k(type, name)); + if (!row) { + throw Object.assign(new Error(`No pending draft exists for ${type}/${name}.`), { code: 'NO_DRAFT', status: 404 }); + } + return { type, name, item: { ...row, _draft: true, _diagnostics: verdictOf(row) } }; + }), + save: vi.fn(async (type: string, name: string, item: unknown, opts?: Record) => { + const body = JSON.parse(JSON.stringify(item)) as Record; + server.saves += 1; + if (opts?.mode === 'draft') server.drafts.set(k(type, name), body); + else server.active.set(k(type, name), body); + return { type, name, item: body }; + }), + publish: vi.fn(async () => ({ success: true })), + reset: vi.fn(async () => ({})), + }; +}); + +vi.mock('./useMetadata', async (importOriginal) => { + const mod = await importOriginal(); + return { + ...mod, + useMetadataClient: () => mockClient, + useMetadataTypes: () => ({ + entries: [{ type: 'page', name: 'page', label: 'Page', allowEnvironmentOverlay: true, schema: { required: [] } }], + }), + }; +}); + +import { MetadataResourceEditPage } from './ResourceEditPage'; +import { registerMetadataPreview, getMetadataPreview } from './preview-registry'; + +/** Canvas stand-in: prints the server diagnostics the page hands a preview, and dirties the draft on demand. */ +function StubCanvas({ + diagnostics, + onPatch, +}: { + diagnostics?: Array<{ path?: string; message: string; severity?: string }>; + onPatch?: (patch: Record) => void; +}) { + return ( + <> +
{JSON.stringify(diagnostics ?? [])}
+ + + ); +} + +const realPage = getMetadataPreview('page'); + +const PAGE: Row = { + name: 'home', + label: 'Home', + type: 'home', + template: 'default', + regions: [{ name: 'main', components: [{ type: 'text', id: 'b1' }] }], +}; + +beforeEach(() => { + server.active.clear(); + server.drafts.clear(); + server.saves = 0; + for (const fn of Object.values(mockClient)) (fn as unknown as { mockClear: () => void }).mockClear(); + registerMetadataPreview('page', StubCanvas as never); +}); + +afterEach(() => { + cleanup(); + if (realPage) registerMetadataPreview('page', realPage); + window.history.replaceState(null, '', '/'); +}); + +function renderEditor() { + window.history.replaceState(null, '', '/metadata/page/home'); + render( + + + , + ); +} + +const canvasDiagnostics = () => + JSON.parse(screen.getByTestId('canvas-diagnostics').textContent ?? '[]') as Array<{ message: string; severity?: string }>; + +describe('MetadataResourceEditPage — a pending draft is judged by its own server verdict (objectstack#26707)', () => { + it('the canvas receives the draft read\'s refusal although the published body is clean', async () => { + server.active.set('page/home', PAGE); + server.drafts.set('page/home', { ...PAGE, label: 'broken' }); + renderEditor(); + + await waitFor( + () => expect(canvasDiagnostics()).toContainEqual(expect.objectContaining({ message: REFUSAL.message, severity: 'error' })), + { timeout: 8000 }, + ); + }); + + it('the verdict follows the draft: the refresh after an autosave of a fixed draft clears it', async () => { + server.active.set('page/home', PAGE); + server.drafts.set('page/home', { ...PAGE, label: 'broken' }); + renderEditor(); + await waitFor(() => expect(canvasDiagnostics().map((d) => d.message)).toContain(REFUSAL.message), { timeout: 8000 }); + + fireEvent.click(screen.getByRole('button', { name: 'fix the draft' })); + + await waitFor(() => expect(server.saves).toBe(1), { timeout: 8000 }); + await waitFor(() => expect(canvasDiagnostics().map((d) => d.message)).not.toContain(REFUSAL.message), { timeout: 8000 }); + }); + + it('control: with no pending draft the published verdict stands', async () => { + server.active.set('page/home', PAGE); + renderEditor(); + + await waitFor(() => expect(mockClient.layered).toHaveBeenCalled(), { timeout: 8000 }); + await screen.findByTestId('canvas-diagnostics', undefined, { timeout: 8000 }); + expect(canvasDiagnostics().map((d) => d.message)).not.toContain(REFUSAL.message); + }); +}); diff --git a/ui/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx b/ui/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx index b37a3b2c213..e0f7e261594 100644 --- a/ui/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx +++ b/ui/packages/app-shell/src/views/metadata-admin/ResourceEditPage.tsx @@ -146,6 +146,12 @@ import { useRegisteredMetadataDefaultInspector } from './default-inspector-regis import { useMetadataLocale, t, tFormat, translateMetadataType, translateValidationMessage } from './i18n.js'; import { JsonSourceEditor } from './JsonSourceEditor.js'; import { validateMetadataDraft, hasClientValidator, type DraftMode } from './clientValidation.js'; +import { + draftEnvelopeDiagnostics, + editedItemDiagnostics, + withServerOnlyErrors, + type ServerDiagnostics, +} from './draftVerdict.js'; import { describeIssuePath, issueSlicePath, issueSliceFingerprint } from './issuePath.js'; import { buildCreateModeBody, seedStructuralDefaults, unsetSeedPlaceholders } from './createBody.js'; import { errorCodeIs, errorCodeIsAnyOf } from '@object-ui/types'; @@ -750,6 +756,18 @@ function MetadataResourceEditPageImpl({ () => (createMode && !createDirty ? [] : issues), [createMode, createDirty, issues], ); + // Per-item draft pending publish (mode=draft saves land here). + // When non-null, the editor is "viewing the draft" and we surface + // Publish / Discard-draft actions. + const [hasDraft, setHasDraft] = React.useState(false); + // objectstack#26707 — the pending draft's own `_diagnostics`, kept from each + // draft read (load, save, publish) before its decorations are stripped. + const [draftDiagnostics, setDraftDiagnostics] = React.useState(undefined); + // The server verdict on the body on screen (see `draftVerdict.ts`). + const serverDiagnostics = React.useMemo( + () => editedItemDiagnostics({ hasDraft, draftDiagnostics, layered }), + [hasDraft, draftDiagnostics, layered], + ); // Server-computed diagnostics handed to a canvas Preview (e.g. the flow // Problems panel + on-canvas badges). Errors prefer the live client-side Zod // issues when a client validator exists (so they track every keystroke); @@ -758,12 +776,13 @@ function MetadataResourceEditPageImpl({ const previewDiagnostics = React.useMemo< Array<{ path?: string; message: string; severity: 'error' | 'warning' }> >(() => { - const diag = (layered as any)?._diagnostics as - | { errors?: Array<{ path: string; message: string }>; warnings?: Array<{ path: string; message: string }> } - | undefined; - const errs = hasClientValidator(type, draftMode) - ? displayIssues.map((i) => ({ path: i.path, message: translateValidationMessage(i.message, locale) })) - : (diag?.errors ?? []).map((i) => ({ path: i.path, message: translateValidationMessage(i.message, locale) })); + // objectstack#26707 — the verdict on the body on screen: the pending + // draft's (which carries what its publish would refuse) while one is held. + const diag = serverDiagnostics; + const errs = (hasClientValidator(type, draftMode) + ? withServerOnlyErrors(displayIssues, diag?.errors) + : (diag?.errors ?? []) + ).map((i) => ({ path: i.path, message: translateValidationMessage(i.message, locale) })); const warns = (diag?.warnings ?? []).map((i) => ({ path: i.path, message: translateValidationMessage(i.message, locale), @@ -772,11 +791,7 @@ function MetadataResourceEditPageImpl({ ...errs.map((e) => ({ path: e.path || undefined, message: e.message, severity: 'error' as const })), ...warns.map((w) => ({ path: w.path || undefined, message: w.message, severity: 'warning' as const })), ]; - }, [layered, displayIssues, type, locale, draftMode]); - // Per-item draft pending publish (mode=draft saves land here). - // When non-null, the editor is "viewing the draft" and we surface - // Publish / Discard-draft actions. - const [hasDraft, setHasDraft] = React.useState(false); + }, [serverDiagnostics, displayIssues, type, locale, draftMode]); const [publishing, setPublishing] = React.useState(false); // Bumped by destructive operations (rollback / discard-draft) to // force the load effect to refetch layered + draft state. @@ -1166,10 +1181,14 @@ function MetadataResourceEditPageImpl({ if (cancelled) return; const draftResp = draftRead.answer; setLayered(lay); + // objectstack#26707 — a pending draft is judged by its own read, whose + // verdict carries what its publish would refuse. + const loadDraftDiagnostics = extractDraftBody(draftResp) ? draftEnvelopeDiagnostics(draftResp) : undefined; + setDraftDiagnostics(loadDraftDiagnostics); // Surface server-computed load-time validation errors as inline // SchemaForm issues — operators see what's wrong with the // saved metadata immediately, not just on the next Save round-trip. - const loadDiag = (lay as any)?._diagnostics as + const loadDiag = loadDraftDiagnostics ?? (lay as any)?._diagnostics as | { valid: boolean; errors?: Array<{ path: string; message: string }> } | undefined; if (loadDiag && loadDiag.valid === false && Array.isArray(loadDiag.errors)) { @@ -1781,6 +1800,7 @@ function MetadataResourceEditPageImpl({ if (!layerless) setLayered(lay); const draftReal = extractDraftBody(draftResp); setHasDraft(!!draftReal); + setDraftDiagnostics(draftReal ? draftEnvelopeDiagnostics(draftResp) : undefined); // The served draft is the whole document the save just stored (see the // load effect, objectui#10765): take it as-is. ⛔ Not spread over // `effective` — that is the PUBLISHED layer, and a spread would bring a @@ -2015,6 +2035,7 @@ function MetadataResourceEditPageImpl({ setLayered(lay); const draftReal = extractDraftBody(draftResp); setHasDraft(!!draftReal); + setDraftDiagnostics(draftReal ? draftEnvelopeDiagnostics(draftResp) : undefined); // After a publish the promoted body IS `effective` and the draft row is // normally gone; a draft that does remain is a whole document and is // taken as-is (objectui#10765). ⛔ Not spread over `effective`: the @@ -2077,6 +2098,7 @@ function MetadataResourceEditPageImpl({ setDraft(fresh); draftSnapshotRef.current = fresh; setHasDraft(false); + setDraftDiagnostics(undefined); // objectui#12303 — the discard is said, naming what it discarded, through // the console's shared toast (`sonner`, the one Studio's publish uses). // Only here, after the DELETE and the re-read landed: a refused discard @@ -2847,27 +2869,20 @@ function MetadataResourceEditPageImpl({ // a structural problem without saving first. The same // errors are also threaded into SchemaForm as `issues` // and rendered inline next to each broken field. - const diag = (layered as any)?._diagnostics as - | { - valid: boolean; - errors?: Array<{ path: string; message: string }>; - warnings?: Array<{ path: string; message: string }>; - } - | undefined; + // objectstack#26707 — the pending draft's verdict while one is held. + const diag = serverDiagnostics; // When client-side Zod validation is available for this // type, drive the error portion of the banner from the // live `issues` state instead of the stale load-time // diagnostics, so it stays in sync with every keystroke. // Warnings remain server-sourced (Zod doesn't model them). - const liveErrors = hasClientValidator(type, draftMode) - ? issues.map((i) => ({ - path: i.path, - message: translateValidationMessage(i.message, locale), - })) - : (diag?.errors ?? []).map((i) => ({ - ...i, - message: translateValidationMessage(i.message, locale), - })); + const liveErrors = (hasClientValidator(type, draftMode) + ? withServerOnlyErrors(issues, diag?.errors) + : (diag?.errors ?? []) + ).map((i) => ({ + path: i.path, + message: translateValidationMessage(i.message, locale), + })); const liveValid = hasClientValidator(type, draftMode) ? liveErrors.length === 0 : diag?.valid !== false; diff --git a/ui/packages/app-shell/src/views/metadata-admin/draftVerdict.test.ts b/ui/packages/app-shell/src/views/metadata-admin/draftVerdict.test.ts new file mode 100644 index 00000000000..2ae176be85b --- /dev/null +++ b/ui/packages/app-shell/src/views/metadata-admin/draftVerdict.test.ts @@ -0,0 +1,31 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect } from 'vitest'; +import { draftEnvelopeDiagnostics, editedItemDiagnostics, withServerOnlyErrors } from './draftVerdict'; + +const refused = { + valid: false, + errors: [{ path: 'edges.1.condition', message: 'invalid CEL predicate', code: 'expression-invalid' }], +}; + +describe('draftVerdict (objectstack#26707)', () => { + it('reads `_diagnostics` off a served draft envelope, before the strip', () => { + expect(draftEnvelopeDiagnostics({ type: 'flow', name: 'f', item: { name: 'f', _draft: true, _diagnostics: refused } })).toBe(refused); + expect(draftEnvelopeDiagnostics({ type: 'flow', name: 'f', item: { name: 'f' } })).toBeUndefined(); + expect(draftEnvelopeDiagnostics(null)).toBeUndefined(); + }); + + it('a pending draft is judged by its own verdict; without one the layered verdict stands', () => { + const layered = { _diagnostics: { valid: true } }; + expect(editedItemDiagnostics({ hasDraft: true, draftDiagnostics: refused, layered })).toBe(refused); + expect(editedItemDiagnostics({ hasDraft: false, draftDiagnostics: refused, layered })).toEqual({ valid: true }); + expect(editedItemDiagnostics({ hasDraft: true, draftDiagnostics: undefined, layered })).toEqual({ valid: true }); + }); + + it('adds the server errors the client parse did not raise, once', () => { + const client = [{ path: 'label', message: 'Required' }]; + const server = [{ path: 'label', message: 'Required' }, ...refused.errors]; + expect(withServerOnlyErrors(client, server)).toEqual([client[0], refused.errors[0]]); + expect(withServerOnlyErrors(client, undefined)).toEqual(client); + }); +}); diff --git a/ui/packages/app-shell/src/views/metadata-admin/draftVerdict.ts b/ui/packages/app-shell/src/views/metadata-admin/draftVerdict.ts new file mode 100644 index 00000000000..7c8b6868704 --- /dev/null +++ b/ui/packages/app-shell/src/views/metadata-admin/draftVerdict.ts @@ -0,0 +1,77 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The server's verdict on the item this editor is editing (objectstack#26707). + * + * The edit page holds two server reads of one item: the layered read, whose + * `_diagnostics` judge the PUBLISHED body, and the draft read, whose + * `_diagnostics` judge the pending draft — the body the editor is actually + * showing. Since objectstack#26707 the draft read's verdict is the publish's + * own: the runtime authoring gate's refusal for that draft (`expression-invalid` + * on a flow edge, a summary field with no operations, …), each entry carrying + * the rule id as its `code` and the publish's message. So while a draft is + * pending, the draft's verdict is the one to show; the layered one describes a + * body the author is not looking at, and read `valid: true` for every draft the + * publish then refused. + * + * ⛔ No verdict is computed here. The rules live on the server, run once there, + * and this module only chooses which served verdict applies and merges it with + * the live client parse. + */ + +/** One `_diagnostics` entry, as served (`MetadataValidationResult`). */ +export interface ServerDiagnosticEntry { + path: string; + message: string; + code?: string; +} + +/** A served `_diagnostics` envelope. */ +export interface ServerDiagnostics { + valid: boolean; + errors?: ServerDiagnosticEntry[]; + warnings?: Array<{ path: string; message: string }>; +} + +/** + * The `_diagnostics` of a served draft envelope (`{ type, name, item }`), read + * BEFORE `extractDraftBody` strips the read decorations off the body. + * `undefined` when there is no pending draft or it carries none. + */ +export function draftEnvelopeDiagnostics(resp: unknown): ServerDiagnostics | undefined { + if (!resp || typeof resp !== 'object') return undefined; + const item = (resp as { item?: unknown }).item; + if (!item || typeof item !== 'object') return undefined; + const diagnostics = (item as { _diagnostics?: unknown })._diagnostics; + if (!diagnostics || typeof diagnostics !== 'object') return undefined; + return diagnostics as ServerDiagnostics; +} + +/** + * The verdict that describes the body on screen: the pending draft's when the + * editor holds one, else the layered read's. + */ +export function editedItemDiagnostics(args: { + hasDraft: boolean; + draftDiagnostics: ServerDiagnostics | undefined; + layered: unknown; +}): ServerDiagnostics | undefined { + if (args.hasDraft && args.draftDiagnostics) return args.draftDiagnostics; + const layered = args.layered as { _diagnostics?: ServerDiagnostics } | null | undefined; + return layered?._diagnostics; +} + +/** + * The errors to show when a live client parse exists: the client's own (they + * track every keystroke), plus each server error the client did not raise — + * the publish-time rules the client parse does not run. An entry the client + * already shows at the same path with the same message is not shown twice. + */ +export function withServerOnlyErrors( + clientErrors: readonly T[], + serverErrors: readonly ServerDiagnosticEntry[] | undefined, +): Array { + const seen = new Set(clientErrors.map((e) => `${e.path}\n${e.message}`)); + const extra = (serverErrors ?? []).filter((e) => !seen.has(`${e.path}\n${e.message}`)); + return [...clientErrors, ...extra]; +}