From b74652f2166e9d3009e7730d10a655e71c082941 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 17:00:32 +0000 Subject: [PATCH 1/5] fix(metadata-protocol): a draft's _diagnostics reports what its publish will refuse The draft read folds the runtime authoring gate's verdict on the stored draft body into _diagnostics: valid:false, each refused rule id as an entry's code, the publish's own message. The publish gate's body is split into judgeRuntimeAuthoringRules (the verdict) and assertRuntimeAuthoringRules (the verdict plus the throw), so the draft read and the publish read one judgement. The draft save is unchanged. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5oVR19x7NPLf4YzCzj --- .../src/metadata-diagnostics.ts | 63 +++ .../src/protocol-authoring-gate.ts | 93 +++- .../protocol.draft-publish-verdict.test.ts | 442 ++++++++++++++++++ packages/metadata-protocol/src/protocol.ts | 79 +++- 4 files changed, 659 insertions(+), 18 deletions(-) create mode 100644 packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts 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..43b9e78d02e --- /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 }) { + if (table === 'sys_metadata_history') { + return historyRows.filter((h) => matchesHistory(h, opts.where)); + } + return Array.from(rows.values()).filter((r) => matchesMetadataWhere(r, opts.where)); + }, + 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.ts b/packages/metadata-protocol/src/protocol.ts index 3b4476a1f96..72bb8778480 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -216,6 +216,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 @@ -359,6 +360,7 @@ import { // [#25467 cut 8] The runtime authoring gate and the restored-body doors. import { assertRuntimeAuthoringRules, + draftPublishVerdict, refuseRestoredBodyTheSaveDoorRefuses, restoredBodyDerivation, restoredBodyWriter, @@ -7107,6 +7109,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. @@ -10701,6 +10749,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 @@ -10848,7 +10915,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) { @@ -10874,6 +10941,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, @@ -10909,7 +10978,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 @@ -10929,10 +11001,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 From 92a9daabed9fdda1b4299a0f59492e6dd0c759b1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 17:11:09 +0000 Subject: [PATCH 2/5] fix(app-shell): the edit page shows a pending draft's own server verdict 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; the page takes it whenever it holds a draft, and merges its server-only errors with the live client parse. Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5oVR19x7NPLf4YzCzj --- ...ditPage.draftPublishVerdict-26707.test.tsx | 195 ++++++++++++++++++ .../views/metadata-admin/ResourceEditPage.tsx | 71 ++++--- .../views/metadata-admin/draftVerdict.test.ts | 31 +++ .../src/views/metadata-admin/draftVerdict.ts | 77 +++++++ 4 files changed, 346 insertions(+), 28 deletions(-) create mode 100644 ui/packages/app-shell/src/views/metadata-admin/ResourceEditPage.draftPublishVerdict-26707.test.tsx create mode 100644 ui/packages/app-shell/src/views/metadata-admin/draftVerdict.test.ts create mode 100644 ui/packages/app-shell/src/views/metadata-admin/draftVerdict.ts 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]; +} From 5ae58ed5c8a893d66d188acfea3e0f0014226fec Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 17:12:28 +0000 Subject: [PATCH 3/5] chore(changeset): draft diagnostics carry the publish verdict Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5oVR19x7NPLf4YzCzj --- .../26707-draft-diagnostics-publish-verdict.md | 15 +++++++++++++++ ui/.changeset/26707-draft-server-verdict.md | 10 ++++++++++ 2 files changed, 25 insertions(+) create mode 100644 .changeset/26707-draft-diagnostics-publish-verdict.md create mode 100644 ui/.changeset/26707-draft-server-verdict.md 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/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 From 6f54ceb949f8ade29d6a4b14afc1256dcff37439 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 17:34:38 +0000 Subject: [PATCH 4/5] test(metadata-protocol): the served draft read's parity includes its publish verdict Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5oVR19x7NPLf4YzCzj --- .../src/protocol.item-execution-read.test.ts | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) 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' })]); From d99646211cb38c18efacf6b49e8a3e74b56cff82 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 27 Oct 2026 18:35:14 +0000 Subject: [PATCH 5/5] test(metadata-protocol): the draft verdict harness honours the caller's bound and joins the engine-double ledger Co-authored-by: Claude Claude-Session: https://claude.ai/code/session_01FEkQ5oVR19x7NPLf4YzCzj --- .../src/protocol.draft-publish-verdict.test.ts | 10 +++++----- scripts/engine-double-contract.pinned.json | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts b/packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts index 43b9e78d02e..8edf32595bd 100644 --- a/packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts +++ b/packages/metadata-protocol/src/protocol.draft-publish-verdict.test.ts @@ -143,11 +143,11 @@ function makeStubEngine() { } return findRow(opts.where)?.row ?? null; }, - async find(table: string, opts: { where: Record }) { - if (table === 'sys_metadata_history') { - return historyRows.filter((h) => matchesHistory(h, opts.where)); - } - return Array.from(rows.values()).filter((r) => matchesMetadataWhere(r, opts.where)); + 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' }; diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 7b73272fea7..614dca1e63c 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",