From c06d7be3cfae92129bf63c91fc293d2b25a43e79 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 02:06:04 +0000 Subject: [PATCH 1/2] fix(app-shell): the pending-changes drill-in leaves the protection envelope out of its diff A new object draft listed the registry stamp `_packageId` under "Also changed:" as if the author had typed it. `computeChangeDetail` now drops the ADR-0010 envelope keys, read off the spec's `MetadataProtectionFields`, from both bodies before it compares them, so every list and the no-change mark read the same envelope-free pair. The fetched bodies keep the envelope, so the security-posture lint still reads them as served. Claude-Session: https://claude.ai/code/session_016v3WXCXoatAw13mUAHxwJB Co-authored-by: Claude --- .../src/preview/DraftChangesPanel.tsx | 37 +++- ...gesPanel.protectionEnvelope-12594.test.tsx | 159 ++++++++++++++++++ 2 files changed, 190 insertions(+), 6 deletions(-) create mode 100644 packages/app-shell/src/preview/__tests__/DraftChangesPanel.protectionEnvelope-12594.test.tsx diff --git a/packages/app-shell/src/preview/DraftChangesPanel.tsx b/packages/app-shell/src/preview/DraftChangesPanel.tsx index eb865e1615..2fef6f5764 100644 --- a/packages/app-shell/src/preview/DraftChangesPanel.tsx +++ b/packages/app-shell/src/preview/DraftChangesPanel.tsx @@ -73,7 +73,7 @@ import { useObjectTranslation } from '@object-ui/i18n'; import { fetchPendingDrafts } from './usePendingDrafts.js'; import { canonicalMetaUrlType } from '@objectstack/spec/shared'; import { resolveI18nLabel, type I18nLabel } from '@objectstack/spec/ui'; -import { stripReadDecorations } from '@objectstack/spec/kernel'; +import { MetadataProtectionFields, 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` @@ -206,9 +206,10 @@ async function publishedNamesOf(type: string): Promise> { * 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. + * copy. The ADR-0010 protection envelope is deliberately NOT on it, and this + * body keeps it: the security-posture lint reads these same bodies as served. + * The review diff leaves the envelope out where it compares, in + * `computeChangeDetail` (objectui#12594). */ function unwrapItem(payload: unknown): Record | null { if (!payload || typeof payload !== 'object') return null; @@ -272,12 +273,36 @@ function isEmptyChangeDetail(detail: EntryChangeDetail): boolean { return fieldRows + keys.added.length + keys.changed.length + keys.removed.length === 0; } +/** + * The ADR-0010 protection envelope (`_lock`, `_packageId`, `_provenance`, …), + * read off the spec's own declaration so no key list lives here. The review + * diff compares what the author wrote, and none of these keys is that: + * `_packageId`, `_packageVersion` and `_provenance` are stamped by the + * registry, and the `_lock*` family is set only by a code package's loader — + * the metadata API refuses a save that would set, change or remove a lock + * (ADR-0010 §3.5). So on a NEW item, whose every key reads as added, the + * package stamp was listed under "Also changed:" as if the author had typed + * it (objectui#12594). + */ +const PROTECTION_ENVELOPE_KEYS: readonly string[] = Object.freeze(Object.keys(MetadataProtectionFields)); + +/** `body` without its top-level protection envelope; a copy, never a mutation. */ +function withoutProtectionEnvelope(body: Record): Record { + const rest = { ...body }; + for (const key of PROTECTION_ENVELOPE_KEYS) delete rest[key]; + return rest; +} + /** * What publishing this draft actually changes, computed client-side from the * published body (null when the item is NEW) and the pending draft body. * `fields` gets the dedicated designer diff; every other top-level key is * compared wholesale — enough to answer "which parts of this item move". * + * Both bodies lose their protection envelope first (above), here and nowhere + * else, so every list this returns and the no-change mark read the same + * envelope-free pair. + * * Why not `computeDiffRows` from `LayeredDiff.tsx`, which splits top-level keys * the same way: that module imports the metadata-admin string table * (`./i18n.js`) at module scope, and this panel sits on the console's EAGER @@ -288,8 +313,8 @@ export function computeChangeDetail( published: Record | null, draft: Record | null, ): EntryChangeDetail { - const pub = published ?? {}; - const cur = draft ?? {}; + const pub = withoutProtectionEnvelope(published ?? {}); + const cur = withoutProtectionEnvelope(draft ?? {}); let fields: EntryChangeDetail['fields'] = null; if (pub.fields != null || cur.fields != null) { const d = diffFields(pub.fields, cur.fields); diff --git a/packages/app-shell/src/preview/__tests__/DraftChangesPanel.protectionEnvelope-12594.test.tsx b/packages/app-shell/src/preview/__tests__/DraftChangesPanel.protectionEnvelope-12594.test.tsx new file mode 100644 index 0000000000..ecd5655d8f --- /dev/null +++ b/packages/app-shell/src/preview/__tests__/DraftChangesPanel.protectionEnvelope-12594.test.tsx @@ -0,0 +1,159 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * The publish-review drill-in lists what the AUTHOR wrote, never the ADR-0010 + * protection envelope (objectui#12594). + * + * ## The defect + * + * A served item carries the envelope the spec declares in + * `MetadataProtectionFields`: the registry's stamps (`_packageId`, + * `_packageVersion`, `_provenance`) and the loader's lock family (`_lock`, + * `_lockReason`, `_lockSource`, `_lockDocsUrl`). `METADATA_READ_DECORATIONS` + * leaves them on the body on purpose, so they reached the diff. On a NEW item + * there is no published body, every key the draft carries reads as added, and + * the sheet listed `_packageId` under "Also changed:" beside the author's own + * keys. + * + * ## The fixtures are derived, not listed + * + * The draft carries EVERY key `Object.keys(MetadataProtectionFields)` names, so + * a key the spec adds to the envelope later is covered here without an edit, + * and a local copy of the list cannot drift from the one the panel reads. + * + * ## The controls + * + * - `label` is an ordinary authored key: it MUST still list, which proves the + * diff ran and the strip took only the envelope. + * - `_unpublished` is a key `AppSchema` declares OUTSIDE the envelope that + * starts with an underscore. It MUST still list, which proves the strip is + * the spec's envelope and not an underscore-prefix filter. + */ + +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'; +import { MetadataProtectionFields } from '@objectstack/spec/kernel'; + +vi.mock('@object-ui/i18n', async (importOriginal) => { + const mod = await importOriginal(); + return { + ...mod, + useObjectTranslation: () => ({ + t: (_k: string, o?: { defaultValue?: string; count?: number }) => + (o?.defaultValue ?? _k).replace('{{count}}', String(o?.count ?? '')), + }), + }; +}); + +import { DraftChangesPanel, computeChangeDetail } from '../DraftChangesPanel'; + +afterEach(() => { + vi.restoreAllMocks(); +}); + +const ENVELOPE_KEYS = Object.keys(MetadataProtectionFields); + +/** A value for every envelope key, as a package-owned served draft wears them. */ +const ENVELOPE: Record = Object.fromEntries( + ENVELOPE_KEYS.map((key) => [key, key === '_lock' ? 'full' : `${key}-value`]), +); + +type Body = Record; + +/** + * Routes for one entry. `published: null` makes it NEW: the published list + * does not name it and the by-name published read 404s. + */ +function mockRoutes(type: string, name: string, draft: Body, published: Body | null) { + global.fetch = vi.fn(async (input: RequestInfo | URL) => { + const url = String(input); + const ok = (body: unknown) => ({ ok: true, status: 200, json: async () => body }); + const notFound = { ok: false, status: 404, json: async () => ({}) }; + if (url.includes('/_drafts')) return ok([{ type, name, packageId: 'com.test.crmext' }]); + if (url.includes('state=draft')) return ok({ type, name, item: { ...draft, _draft: true } }); + if (url.includes(`/meta/${type}/${name}`)) return published ? ok({ type, name, item: published }) : notFound; + if (new RegExp(`/meta/${type}(\\?|$)`).test(url)) return ok(published ? [{ name }] : []); + return notFound; + }) as unknown as typeof fetch; +} + +async function openEntryDetail() { + render( {}} packageId="com.test.crmext" onPublish={vi.fn()} />); + fireEvent.click(await screen.findByTestId('draft-entry-toggle')); +} + +/** The text of every key row the drill-in rendered, `+ name` / `~ name` / `− name`. */ +function renderedRows(): string[] { + return [...screen.getByTestId('draft-entry-detail').querySelectorAll('p.font-mono')].map((p) => + (p.textContent ?? '').trim(), + ); +} + +describe('DraftChangesPanel — the protection envelope never lists as authored (objectui#12594)', () => { + it('reads a non-empty envelope off the spec', () => { + // Guards every assertion below: an empty key list would make them vacuous. + expect(ENVELOPE_KEYS).toContain('_packageId'); + expect(ENVELOPE_KEYS).toContain('_lock'); + }); + + it("lists no envelope key on a NEW object's drill-in, and still lists the authored keys", async () => { + mockRoutes( + 'object', + 'crmext_visit', + { name: 'crmext_visit', label: 'Visit', fields: { subject: { type: 'text' } }, ...ENVELOPE }, + null, + ); + await openEntryDetail(); + + // CONTROL: the authored key and the field are listed, so the diff ran. + await waitFor(() => expect(renderedRows()).toContain('+ label')); + expect(renderedRows()).toContain('+ subject'); + + const text = screen.getByTestId('draft-entry-detail').textContent ?? ''; + for (const key of ENVELOPE_KEYS) expect(text).not.toContain(key); + }); + + it('still lists a declared underscore key that is not in the envelope', async () => { + // `_unpublished` is declared by `AppSchema`, beside the envelope it spreads. + expect(ENVELOPE_KEYS).not.toContain('_unpublished'); + mockRoutes('app', 'crmext_app', { name: 'crmext_app', label: 'CRM', _unpublished: true, ...ENVELOPE }, null); + await openEntryDetail(); + + await waitFor(() => expect(renderedRows()).toContain('+ _unpublished')); + expect(renderedRows()).toContain('+ label'); + const text = screen.getByTestId('draft-entry-detail').textContent ?? ''; + for (const key of ENVELOPE_KEYS) expect(text).not.toContain(key); + }); + + it('reads an UPDATE whose bodies differ only in the envelope as unchanged', async () => { + const authored = { name: 'crmext_visit', label: 'Visit', fields: { subject: { type: 'text' } } }; + mockRoutes( + 'object', + 'crmext_visit', + { ...authored, ...ENVELOPE }, + { ...authored, _lock: 'none', _packageId: 'com.test.other', _provenance: 'org' }, + ); + await openEntryDetail(); + + await waitFor(() => expect(screen.getByText(/No differences detected/)).toBeInTheDocument(), { + timeout: 4000, + }); + expect(screen.queryByTestId('draft-entry-detail')).not.toBeInTheDocument(); + }); + + it('leaves the envelope out of every list computeChangeDetail returns', () => { + const detail = computeChangeDetail(null, { label: 'Visit', fields: { subject: { type: 'text' } }, ...ENVELOPE }); + expect(detail.keys).toEqual({ added: ['label'], changed: [], removed: [] }); + expect(detail.fields?.added).toEqual(['subject']); + + // The REMOVED direction too: a published body with the envelope against a + // draft without one lists nothing. + expect(computeChangeDetail({ label: 'Visit', ...ENVELOPE }, { label: 'Visit' }).keys).toEqual({ + added: [], + changed: [], + removed: [], + }); + }); +}); From 16a279685448f7f5a8c1992bd1ac44821e03b410 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 16 Oct 2026 02:06:18 +0000 Subject: [PATCH 2/2] chore(changeset): note the drill-in protection-envelope fix Claude-Session: https://claude.ai/code/session_016v3WXCXoatAw13mUAHxwJB Co-authored-by: Claude --- .changeset/12594-drill-in-protection-envelope.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/12594-drill-in-protection-envelope.md diff --git a/.changeset/12594-drill-in-protection-envelope.md b/.changeset/12594-drill-in-protection-envelope.md new file mode 100644 index 0000000000..0d647554d9 --- /dev/null +++ b/.changeset/12594-drill-in-protection-envelope.md @@ -0,0 +1,5 @@ +--- +'@object-ui/app-shell': patch +--- + +The pending-changes sheet no longer lists the package stamp as one of the author's own changes (objectui#12594). Expanding a new object draft showed `_packageId` under "Also changed:" beside the keys the author set. The drill-in now leaves out the protection keys every served item carries (`_packageId`, `_packageVersion`, `_provenance` and the `_lock` family), read from `@objectstack/spec`'s `MetadataProtectionFields`. The same applies to an update: a draft that differs from its published version only in those keys reads as "No differences detected". The registry and the package loader set these keys, and the metadata API refuses a save that would change a lock, so none of them is an edit the author can publish. Every other key still lists, an underscore key the spec declares outside that set included (an app's `_unpublished`). Nothing served or saved changes.