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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions .changeset/8181-draft-envelope-read-decoration-strip.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
---
'@object-ui/data-objectstack': patch
'@object-ui/app-shell': patch
---

fix(studio): one draft-envelope reader, and it strips the framework's read decorations

`client.getDraft()` serves a DECORATED body — the draft branch stamps
`_draft: true` and then `decorateMetadataItem` attaches `_diagnostics` for any
type with a registered Zod schema. The spec names both READ-TIME decorations
precisely because a served body "is NOT a valid input to the schema that
produced it until these are removed" (`METADATA_READ_DECORATIONS`).

objectui#7603 taught `ResourceEditPage` to strip them. It could only teach one
site, because `extractDraftBody` existed **four times** — three verbatim copies
plus a hand-rolled one in `ObjectHooksPanel` — and six more consumers unwrapped
the envelope inline. Ten readers, one of which knew the rule.

**The user-visible half.** The pending-changes sheet's per-entry diff compares
the published body against the draft body key by key. Those two reads are
decorated ASYMMETRICALLY — only the draft branch stamps `_draft` — so the sheet
listed `_draft` under "Also changed:" on every entry that has a published
counterpart, and `_diagnostics` alongside it whenever the two read-time verdicts
differed. Framework-internal keys were being presented to the author as their
own edits, on the screen where they decide whether to publish.

**The rest.** Six sites merged a decorated body into a document they then wrote
back through `save(..., { mode: 'draft' })` — the Studio app / page / object /
flow surfaces, the package OWD panel, the object hooks panel, and the
adapter's `updateView`. Today's server absorbs that (it strips read decorations
on ingress, before its own schema gate), so nothing 400s; this is still a client
emitting a body its own spec calls invalid, and the fix belongs at the producer.

The cure is one function rather than ten strips: `extractDraftBody` is now
exported from `@object-ui/data-objectstack`, beside the `getDraft` whose
envelope it decodes. The key list is the spec's exported
`stripReadDecorations` — never a second hand-maintained copy in this repo. The
presence verdict still runs BEFORE the strip, so removing our own annotations
can never turn a served draft into "nothing pending", and the ADR-0010
protection envelope (`_lock`, `_provenance`, `_packageId`, `_packageVersion`)
is deliberately untouched: those keys are declared by the closed schemas.

No schema was loosened, and no gate was taught to tolerate `_diagnostics`.
23 changes: 21 additions & 2 deletions packages/app-shell/src/preview/DraftChangesPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ import { useObjectTranslation } from '@object-ui/i18n';
// refuses, and a faithful copy is exactly the fork that guard exists to prevent.
import { fetchPendingDrafts } from './usePendingDrafts.js';
import { canonicalMetaUrlType } from '@objectstack/spec/shared';
import { stripReadDecorations } from '@objectstack/spec/kernel';
import { diffFields } from '../views/metadata-admin/previews/object-fields-io.js';
// The live `?surface=` channel, and NOT `useSurfaceDeepLink` beside it: this
// import must stay React-only, because that module reaches `nav-selection.js`
Expand Down Expand Up @@ -151,11 +152,29 @@ async function publishedNamesOf(type: string): Promise<Set<string>> {
* Some framework reads wrap the body in a `{ type, name, item }` envelope
* (draft reads do; published reads return the bare body). Unwrap defensively.
*/
/**
* Take the body out of a `/meta` response, decoration-free (objectui#8181).
*
* ⚠️ The strip is not cosmetic here — it is what makes the review diff below
* TRUE. `computeChangeDetail` compares every top-level key of the published
* body against the draft body, and the framework decorates the two reads
* ASYMMETRICALLY: the draft branch stamps `_draft: true` before decorating,
* the published branch does not. So `_draft` differed on every entry that has
* a published counterpart, and the sheet listed it as a key this publish
* changes — a framework-internal key presented to the author as their own
* edit, on the door where they decide whether to publish. `_diagnostics` rides
* the same asymmetry whenever the two bodies' read-time verdicts differ.
*
* The key list is the spec's (`METADATA_READ_DECORATIONS`), never a local
* copy. The ADR-0010 protection envelope is deliberately NOT on it: those keys
* are declared by the closed schemas, so a real change to one of them IS a
* change this diff should report.
*/
function unwrapItem(payload: unknown): Record<string, unknown> | null {
if (!payload || typeof payload !== 'object') return null;
const p = payload as Record<string, unknown>;
if (p.item && typeof p.item === 'object') return p.item as Record<string, unknown>;
return p;
const body = p.item && typeof p.item === 'object' ? (p.item as Record<string, unknown>) : p;
return stripReadDecorations(body) as Record<string, unknown>;
}

async function fetchItemBody(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.

/**
* The publish-review diff reports what the AUTHOR changed, never what the
* framework decorated (objectui#8181).
*
* ## The defect, and why it was live rather than latent
*
* `EntryDetail` fetches the item twice — published, then `?state=draft` — and
* `computeChangeDetail` reports every top-level key whose value differs. The
* framework decorates those two reads ASYMMETRICALLY: the draft branch stamps
* `_draft: true` on the row before handing it to `decorateMetadataItem`, and
* the published branch does not stamp anything. `unwrapItem` took the body
* verbatim, so `_draft` differed on EVERY entry that has a published
* counterpart, and the sheet listed it under "Also changed:" — a
* framework-internal key rendered to the author as one of their own edits, on
* the screen where they decide whether to publish. `_diagnostics` joins it
* whenever the two reads' verdicts differ, which is the normal case for a
* draft that changed anything.
*
* This is the half of objectui#8181 that needed no failure arm and no schema
* gate to be wrong: it is wrong on the happy path, every time, in front of the
* author.
*
* ## Why the fixture carries BOTH reads
*
* ⚠️ Drop the published read (make the entry NEW) and this passes with the
* defect fully present: `computeChangeDetail` short-circuits `pub` to `{}` and
* every key is "changed", so the decoration hides in a list that is expected to
* be long. The published-vs-draft PAIR is the trigger.
*
* ## The control
*
* `label` differs between the two bodies on purpose. It MUST appear under
* "Also changed:" — that is what proves the strip took the framework's keys and
* not the diff itself. Without it, a `unwrapItem` that returned `null` for
* everything would pass every "not.toContain" assertion in this file.
*/

import '@testing-library/jest-dom/vitest';
import * as React from 'react';
import { afterEach, describe, expect, it, vi } from 'vitest';
import { render, screen, fireEvent, waitFor } from '@testing-library/react';

vi.mock('@object-ui/i18n', async (importOriginal) => {
const mod = await importOriginal<typeof import('@object-ui/i18n')>();
return {
...mod,
useObjectTranslation: () => ({
t: (_k: string, o?: { defaultValue?: string; count?: number }) =>
(o?.defaultValue ?? _k).replace('{{count}}', String(o?.count ?? '')),
}),
};
});

import { DraftChangesPanel } from '../DraftChangesPanel';

afterEach(() => {
vi.restoreAllMocks();
});

const PUBLISHED = {
name: 'crmext_visit',
label: 'Visit',
sharingModel: 'private',
fields: { name: { type: 'text' } },
// A published read is decorated too — only the `_draft` stamp is draft-only.
_diagnostics: { valid: true, errors: [] },
};

const DRAFT = {
name: 'crmext_visit',
label: 'Customer Visit', // ← the CONTROL: a real authored change
sharingModel: 'private',
fields: { name: { type: 'text' } },
_diagnostics: { valid: false, errors: [{ path: 'label', message: 'x' }] },
_draft: true,
};

function mockRoutes() {
global.fetch = vi.fn(async (input: RequestInfo | URL) => {
const url = String(input);
const ok = (body: unknown) => ({ ok: true, status: 200, json: async () => body });
if (url.includes('/_drafts')) {
return ok([{ type: 'object', name: 'crmext_visit', packageId: 'com.test.crmext' }]);
}
if (url.includes('state=draft')) {
return ok({ type: 'object', name: 'crmext_visit', item: DRAFT });
}
if (/\/meta\/object\/crmext_visit/.test(url)) {
return ok({ type: 'object', name: 'crmext_visit', item: PUBLISHED });
}
if (/\/meta\/object(\?|$)/.test(url)) return ok([{ name: 'crmext_visit' }]);
return { ok: false, status: 404, json: async () => ({}) };
}) as unknown as typeof fetch;
}

async function openEntryDetail() {
render(
<DraftChangesPanel open onOpenChange={() => {}} packageId="com.test.crmext" onPublish={vi.fn()} />,
);
const toggle = await screen.findByTestId('draft-entry-toggle');
fireEvent.click(toggle);
return screen.findByTestId('draft-entry-detail', undefined, { timeout: 4000 });
}

describe('DraftChangesPanel — read decorations never reach the review diff (objectui#8181)', () => {
it('does not report `_draft` or `_diagnostics` as keys this publish changes', async () => {
mockRoutes();
const detail = await openEntryDetail();

// The CONTROL first: the real authored change IS reported, so the diff ran
// and this harness reaches the changed-keys strip.
await waitFor(() => expect(detail.textContent).toContain('label'));

// …and the framework's own keys are not sitting next to it. `_draft` is the
// deterministic one — the server stamps it on the draft read and never on
// the published read, so before the fix it was named on EVERY entry.
expect(detail.textContent).not.toContain('_draft');
expect(detail.textContent).not.toContain('_diagnostics');
});

it('still reports nothing at all when only the decorations differ', async () => {
// The same body on both sides except for the framework's stamps: the honest
// answer is "the draft matches the published version", and before the fix
// it was "Also changed: _diagnostics, _draft".
global.fetch = vi.fn(async (input: RequestInfo | URL) => {
const url = String(input);
const ok = (body: unknown) => ({ ok: true, status: 200, json: async () => body });
const AUTHORED = { name: 'crmext_visit', label: 'Visit', fields: { name: { type: 'text' } } };
if (url.includes('/_drafts')) {
return ok([{ type: 'object', name: 'crmext_visit', packageId: 'com.test.crmext' }]);
}
if (url.includes('state=draft')) {
return ok({
type: 'object',
name: 'crmext_visit',
item: { ...AUTHORED, _draft: true, _diagnostics: { valid: false, errors: [{ x: 1 }] } },
});
}
if (/\/meta\/object\/crmext_visit/.test(url)) {
return ok({
type: 'object',
name: 'crmext_visit',
item: { ...AUTHORED, _diagnostics: { valid: true, errors: [] } },
});
}
if (/\/meta\/object(\?|$)/.test(url)) return ok([{ name: 'crmext_visit' }]);
return { ok: false, status: 404, json: async () => ({}) };
}) as unknown as typeof fetch;

render(
<DraftChangesPanel open onOpenChange={() => {}} packageId="com.test.crmext" onPublish={vi.fn()} />,
);
fireEvent.click(await screen.findByTestId('draft-entry-toggle'));

await waitFor(
() => expect(screen.getByText(/No differences detected/)).toBeInTheDocument(),
{ timeout: 4000 },
);
// The strip is what produced that verdict, so the detail block — which only
// renders when there IS something to report — must be absent.
expect(screen.queryByTestId('draft-entry-detail')).not.toBeInTheDocument();
});
});
16 changes: 15 additions & 1 deletion packages/app-shell/src/preview/capabilityLint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
* capability may legitimately be provided by another installed package).
*/

import { stripReadDecorations } from '@objectstack/spec/kernel';

interface PendingDraft {
type: string;
name: string;
Expand Down Expand Up @@ -83,9 +85,21 @@ export async function lintDraftCapabilityReferences(
const linted = pending.filter((d) => LINTED_DRAFT_TYPES.has(d.type));
if (linted.length === 0) return [];

// Read decorations do not reach the rule (objectui#8181). MEASURED: neither
// `validateSecurityPosture` nor `validateCapabilityReferences` moves its
// verdict on a decorated body today (control: an object with no
// `sharingModel` fires `security-owd-unset` in both directions), so this is
// defence in depth rather than a fix. It is here because the alternative is
// leaving one more verbatim copy of the unwrap that omits the strip, which
// is the shape objectui#8181 exists to end: the rule should read the
// document the AUTHOR wrote, so a future rule that enumerates keys cannot
// inherit the defect. The key list is the spec's, never a local copy; the
// ADR-0010 protection envelope is not on it and survives.
const unwrap = (raw: unknown): Record<string, unknown> | null => {
const item = (raw as { item?: unknown })?.item ?? raw;
return item && typeof item === 'object' ? (item as Record<string, unknown>) : null;
return item && typeof item === 'object'
? (stripReadDecorations(item) as Record<string, unknown>)
: null;
};

const bodies = await Promise.all(
Expand Down
16 changes: 15 additions & 1 deletion packages/app-shell/src/preview/securityPostureLint.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@
* pull the whole lint bundle onto the eager console graph.
*/

import { stripReadDecorations } from '@objectstack/spec/kernel';

interface PendingDraft {
type: string;
name: string;
Expand Down Expand Up @@ -124,9 +126,21 @@ export async function lintDraftSecurityPosture(
const objects = pending.filter((d) => d.type === 'object');
if (objects.length === 0) return [];

// Read decorations do not reach the rule (objectui#8181). MEASURED: neither
// `validateSecurityPosture` nor `validateCapabilityReferences` moves its
// verdict on a decorated body today (control: an object with no
// `sharingModel` fires `security-owd-unset` in both directions), so this is
// defence in depth rather than a fix. It is here because the alternative is
// leaving one more verbatim copy of the unwrap that omits the strip, which
// is the shape objectui#8181 exists to end: the rule should read the
// document the AUTHOR wrote, so a future rule that enumerates keys cannot
// inherit the defect. The key list is the spec's, never a local copy; the
// ADR-0010 protection envelope is not on it and survives.
const unwrap = (raw: unknown): Record<string, unknown> | null => {
const item = (raw as { item?: unknown })?.item ?? raw;
return item && typeof item === 'object' ? (item as Record<string, unknown>) : null;
return item && typeof item === 'object'
? (stripReadDecorations(item) as Record<string, unknown>)
: null;
};

const bodies = await Promise.all(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@

import * as React from 'react';
import { useNavigate } from 'react-router-dom';
import { stripReadDecorations } from '@objectstack/spec/kernel';
import {
Save,
Loader2,
Expand Down Expand Up @@ -450,8 +451,16 @@ export function PermissionMatrixEditPage({ type, name, packageId, onDraftSaved,
// envelope the display baseline comes from, so the writability verdict
// and the body on screen can never be read from different round trips.
setCodeIsArtifact(isArtifactBackedLayer(lay));
// Read decorations do NOT seed the editor (objectui#8181). `doSave`
// below re-bases on a fresh RAW `layered` read, which drops them — but
// its `.catch(() => null)` arm falls back to this very body and
// spreads it into `client.save`, so a failed layered read used to put
// `_diagnostics` / `_draft` on the wire. Strip at the unwrap, which is
// the one place the served envelope becomes an editable draft.
const draftBody = pendingDraft
? (((pendingDraft as any).item ?? pendingDraft) as PermissionSetDraft)
? (stripReadDecorations(
(pendingDraft as any).item ?? pendingDraft,
) as PermissionSetDraft)
: null;
// Draft wins over the published baseline for display (D6).
const effective: PermissionSetDraft = (draftBody ?? lay?.effective ??
Expand Down
Loading
Loading