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
15 changes: 15 additions & 0 deletions .changeset/record-details-retire-layout-input-3818.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
'@object-ui/plugin-detail': patch
---

`record:details` stops publishing a `layout` key the spec removed and the renderer never honoured

`record:details` declared `layout: enum ['auto','custom']` with `defaultValue: 'auto'` and the description "auto uses the object highlightFields; custom uses explicit sections". None of that was ever implemented. The renderer's only `schema.layout` read tested `'inline'` | `'compact'` — two values the schema never permitted — so both legal values fell through the same ternary and the key selected nothing. `auto` and `custom` have behaved identically for as long as both have existed.

Two directions were wrong with zero diagnostics: `layout: 'auto'` plus explicit `sections` still rendered the sections, and `layout: 'custom'` with no sections silently fell back to the flat body rather than reporting the missing groups. Because the input carried a `defaultValue`, this was not stale documentation — it was the manifest, the generated `sdui-intrinsics.d.ts` and the designer panel actively offering the key. An AI author writing `layout: 'custom'` believed it took effect.

`@objectstack/spec` 17.0.0 removed the property (objectstack#6946, ADR-0087 D2); `17.0.0-rc.6` is pinned here, so the key is already rejected on parse with a named migration message pointing at `os migrate meta --from 16`. This release completes the objectui half of that retirement: the input declaration is gone, and so is the dead `inline`/`compact` branch — the synthesized layout is now the constant it always resolved to.

Nothing that worked stops working. The body-source contract is unchanged and is now the only one declared: **`sections` renders the explicit groups; omitting it falls back to the flat body derived from the object's fields.** That is pinned in both directions, plus the empty-array boundary between them, in `recordDetailsBodySource.test.tsx`.

One gate got sharper on the way through. The parity test's "declares no top-level input the spec does not accept" check read raw `.shape` keys — but an ADR-0087 D2 tombstone stays *in* the shape as a `z.never()`, so a retired key still answers "is this declared?" with yes. That is precisely why this input survived the rc.6 pin bump with every derived gate green. The check now filters tombstoned members out, so it catches the next D2 retirement instead of waving it through.
26 changes: 26 additions & 0 deletions apps/console/src/__tests__/registry-inputs-spec-parity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,32 @@ const UNPUBLISHED_EXEMPTIONS: Record<string, string> = {
'page:card.body':
'Retired upstream by objectstack#5775 / PR objectstack#6281 (ADR-0087 D2 tombstone, converging on the `children` this renderer reads and now publishes); declaring it would publish a key the spec rejects by name — objectui#4027. Listed here only because the pinned @objectstack/spec@17.0.0-rc.5 predates the retirement. Resolves via objectui#3809, not via the pin bump.',

// ── record:details.layout — retired upstream AND withdrawn here (1 key) ───
// The fifth D2 tombstone, and the first one whose objectui half has actually
// landed — so it is here for a DIFFERENT reason than the four above, and the
// difference is worth reading before treating it as more of the same.
//
// Those four are stale-pin cover: the key is still published in this repo and
// the entry says "the pin predates the retirement". This one is the opposite.
// objectui#3818 DELETED the `record:details` `layout` input (the spec's
// `auto` | `custom` semantics were never implemented — the renderer's only
// read tested `inline` | `compact`, values the schema never permitted, so both
// legal values took the same branch and the key selected nothing), which is
// exactly what this gate's forward direction wants. The entry exists because
// the REVERSE direction then demands the key back: `specTopLevelKeys` reads
// raw `Object.keys(shape)`, the ADR-0087 D2 tombstone is still an entry in
// that shape, and so a key the spec rejects by name reads as "declared, and
// you failed to publish it".
//
// That is objectui#3809's blind spot seen from the other side — it predicted a
// false GREEN in the forward direction, and this is the same root cause
// producing a false RED in the reverse one. Both vanish together when #3809
// narrows `specTopLevelKeys` to non-tombstone members; this entry then goes
// stale and `carries no stale unpublished-key exemption` will name it, along
// with the four above. Do not resolve it by re-adding the input.
'record:details.layout':
'Retired upstream by objectstack#6946 (ADR-0087 D2 tombstone) and withdrawn here by objectui#3818 — its published `auto` | `custom` semantics were never implemented, and the spec now rejects the key by name, so publishing it again would teach a key the contract refuses. Unlike the stale-pin entries above this one is live at @objectstack/spec@17.0.0-rc.6: the tombstone stays in `Object.keys(shape)`, so the reverse direction demands a key the forward direction forbids. Resolves via objectui#3809 tombstone recognition, not by declaring the input.',

// `element:record_picker.filter` was the ninth entry here — a real A-class gap
// that fell out of objectui#3808's three-class triage, exempted only because it
// was outside that PR's dispatched scope. objectui#3830 declared the input, so
Expand Down
114 changes: 114 additions & 0 deletions packages/plugin-detail/src/__tests__/recordDetailsBodySource.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

/**
* `record:details` — what chooses the detail body (objectui#3818).
*
* The positive control for the `layout` retirement. `record:details` published
* `layout: enum ['auto','custom']` describing itself as the body selector
* ("auto uses the object highlightFields; custom uses explicit sections"), but
* the renderer's only read tested `'inline'` | `'compact'` — values the schema
* never permitted — so both legal values took the same branch and the key
* selected nothing. @objectstack/spec 17.0.0 removed it (objectstack#6946,
* ADR-0087 D2) and objectui#3818 removes the input and the dead branch here.
*
* Deleting a selector is only safe if the REAL selector is pinned, so this
* file pins the contract that was doing the work all along and is now the
* documented one: **what you author decides the body.** `sections` renders the
* explicit groups (the old `custom`); omitting it falls back to the flat body
* (the old `auto`). Both directions, because the failure this guards is a body
* that silently comes up wrong — or blank — with no diagnostic anywhere.
*
* WHY THERE IS NO "authoring `layout` changes nothing" ASSERTION HERE. It
* would be vacuous, and the measurement that says so is worth recording: the
* deleted branch fed `synthesized.layout`, and `DetailView` never reads
* `layout` at all (no read site in the file, and it does not spread the schema
* onward). So the branch was dead TWICE — retired vocabulary AND ignored
* consumer — and restoring it changes no rendered output. A DOM-equality pin
* would therefore stay green through the exact regression it appears to guard.
* The read point is pinned where it can actually go red: the source-text and
* published-surface assertions in `recordDetailsInputs.spec-parity.test.ts`.
*/

import { describe, it, expect } from 'vitest';
import { render, screen } from '@testing-library/react';
import * as React from 'react';
import { RecordContextProvider } from '@object-ui/react';
import { RecordDetailsRenderer } from '../renderers/record-details';

/**
* No `name` / `title` / `display_name` in the data on purpose: the renderer
* drops the page-H1 title field from the body (`titleCandidates`), and a
* fixture that tripped that would make an absence assertion below pass for the
* wrong reason.
*/
const objectSchema = {
fields: {
phone: { type: 'text', label: 'Phone' },
email: { type: 'text', label: 'Email' },
industry: { type: 'text', label: 'Industry' },
},
};

const data = {
phone: '555-0100',
email: 'ops@acme.test',
industry: 'Manufacturing',
};

const renderDetails = (schema: Record<string, unknown>) =>
render(
<RecordContextProvider
objectName="crm_account"
recordId="A1"
data={data}
objectSchema={objectSchema}
>
<RecordDetailsRenderer schema={schema as any} />
</RecordContextProvider>,
);

describe('record:details — `sections` presence decides the body (#3818)', () => {
it('renders the authored groups when `sections` is present (the old `custom`)', () => {
renderDetails({
sections: [
{ name: 'contact_info', label: 'Contact Info', fields: ['phone', 'email'] },
],
});

// The group heading and its fields are on screen...
expect(screen.getByText('Contact Info')).toBeInTheDocument();
expect(screen.getByText('555-0100')).toBeInTheDocument();
expect(screen.getByText('ops@acme.test')).toBeInTheDocument();

// ...and a field OUTSIDE every section is not, which is the half that
// matters: once `sections` is authored it is the ONLY source of the body.
// `industry` is present in both the object schema and the data, so its
// absence is a decision by `sections`, not missing input.
expect(screen.queryByText('Manufacturing')).not.toBeInTheDocument();
});

it('falls back to the flat field body when `sections` is absent (the old `auto`)', () => {
renderDetails({ fields: ['industry'] });

expect(screen.getByText('Manufacturing')).toBeInTheDocument();
// No section chrome: the flat arm is a single unlabelled group, so the
// heading from the other direction must not appear.
expect(screen.queryByText('Contact Info')).not.toBeInTheDocument();
});

it('an empty `sections` array is not "sections authored" — the flat body still wins', () => {
// The boundary between the two arms. `DetailView` gates the section arm on
// `sections.length > 0` and the flat arm on `!sections?.length`, so the
// empty array must behave as absence rather than blanking the body — the
// blank-page failure mode this block exists to keep closed.
renderDetails({ sections: [], fields: ['industry'] });

expect(screen.getByText('Manufacturing')).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,15 @@
*/

import { describe, it, expect } from 'vitest';
import { readFileSync } from 'node:fs';
import { fileURLToPath } from 'node:url';
import { dirname, join } from 'node:path';
import { ComponentRegistry } from '@object-ui/core';
import { RecordDetailsProps } from '@objectstack/spec/ui';
import '../index';

const SRC_DIR = join(dirname(fileURLToPath(import.meta.url)), '..');

type ShapeCarrier = { shape?: unknown; _def?: { shape?: unknown } };

/** Resolve a Zod object's `.shape` through both spellings, lazy or plain. */
Expand Down Expand Up @@ -78,9 +83,34 @@ function arrayElement(schema: unknown): unknown {
return arr?.element ?? arr?.def?.element ?? arr?._def?.element ?? arr?._def?.type;
}

/** Top-level keys of the spec's `RecordDetailsProps`. */
/** Top-level keys of the spec's `RecordDetailsProps`, INCLUDING tombstones. */
const specTopLevelKeys = (): string[] => shapeKeys(RecordDetailsProps);

/**
* Is this top-level key an ADR-0087 tombstone — declared, but typed `never` so
* every value is rejected with a named migration message?
*
* This distinction is load-bearing, and objectui#3818 is what proved it. A D2
* retirement does NOT delete the key from the shape; it REPLACES the member
* with `z.never()` carrying the "removed in 17.0.0, run `os migrate meta`"
* text. So a retired key still answers `Object.keys(shape)` — which is why
* `layout` survived the 17.0.0-rc.6 pin bump on the published surface below
* with every derived gate green: the gate asked "is this key in the shape",
* the tombstone said yes, and the manifest kept offering an input the spec
* rejects on parse. Filtering tombstones out is what makes the gate mean what
* its name says.
*/
const isTombstoned = (key: string): boolean => {
const member = shapeMember(RecordDetailsProps, key) as
| { _def?: { type?: string }; def?: { type?: string } }
| undefined;
return (member?._def?.type ?? member?.def?.type) === 'never';
};

/** Top-level keys the spec actually ACCEPTS — tombstones removed. */
const specAcceptedTopLevelKeys = (): string[] =>
specTopLevelKeys().filter((key) => !isTombstoned(key));

/** Member keys of one `sections[]` entry, per the spec. */
const specSectionKeys = (): string[] =>
shapeKeys(arrayElement(shapeMember(RecordDetailsProps, 'sections')));
Expand Down Expand Up @@ -191,7 +221,11 @@ describe('record:details — registry inputs vs @objectstack/spec', () => {
});

it('declares no top-level input the spec does not accept', () => {
const allowed = new Set(specTopLevelKeys());
// ACCEPTED keys, not merely DECLARED ones — a tombstone is present in the
// shape and rejects every value (see `isTombstoned`). Reading raw shape
// keys here is what let the retired `layout` input stay published and green
// through the 17.0.0-rc.6 bump (objectui#3818).
const allowed = new Set(specAcceptedTopLevelKeys());
const offSpec = inputs().map((i) => i.name).filter((name) => !allowed.has(name));
expect(offSpec).toEqual([]);
});
Expand Down Expand Up @@ -262,3 +296,80 @@ describe('record:details — registry inputs vs @objectstack/spec', () => {
expect(description).not.toContain('{');
});
});

/**
* objectui#3818 — the retired `layout` key, pinned on the published surface.
*
* `record:details` published `layout: enum ['auto','custom']` with
* `defaultValue: 'auto'` and the description "auto uses the object
* highlightFields; custom uses explicit sections". None of that was ever
* implemented: the renderer's ONLY `schema.layout` read tested `'inline'` |
* `'compact'` — two values the schema never permitted — so both legal values
* fell through the same ternary to `'vertical'` and the key selected nothing.
* A two-value enum whose values do the same thing is a false affordance with
* zero diagnostics: an author writing `layout: 'custom'` believed it took
* effect. @objectstack/spec 17.0.0 removed the key (objectstack#6946,
* ADR-0087 D2) and the maintainer ruling of 2026-08-09 on objectui#3818 is
* REMOVAL on this side too.
*
* Why these pins are not redundant with the derived gate above: a D2 tombstone
* stays IN the shape (see `isTombstoned`), so every key-presence check kept
* passing while the input was still published. The gate is now
* tombstone-aware, and the assertions below name this key so the repair stays
* legible if that helper is ever loosened.
*/
describe('record:details — `layout` is retired, not merely undocumented (#3818)', () => {
it('the spec REJECTS `layout`, with the ADR-0087 migration message', () => {
// The premise, checked first: if upstream ever un-retires the key, this
// fails before the negative pins below start guarding a dead rule.
expect(specTopLevelKeys()).toContain('layout'); // tombstone is still declared
expect(isTombstoned('layout')).toBe(true);
expect(specAcceptedTopLevelKeys()).not.toContain('layout');

// A rejection case, so the assertion set is the envelope and not a bare
// "it failed": path + code + the named migration text. Both formerly
// published values AND both values the dead renderer branch tested — all
// four are unauthorable now, which is the whole point.
for (const value of ['auto', 'custom', 'inline', 'compact']) {
const parsed = RecordDetailsProps.safeParse({ layout: value });
expect(parsed.success).toBe(false);
expect(parsed.error?.issues.map((i) => i.path.join('.'))).toContain('layout');
expect(parsed.error?.issues.map((i) => i.code)).toContain('invalid_type');
expect(parsed.error?.issues[0]?.message).toMatch(/removed in @objectstack\/spec 17\.0\.0/);
}
});

it('the published surface offers no `layout` input', () => {
// Non-empty FIRST — `inputs()` returning `[]` would satisfy every
// absence assertion below for the wrong reason (the vacuous-green trap
// the `hideFields` block above documents from objectui#3808's reverse run).
expect(inputs().length).toBeGreaterThan(0);
expect(inputs().map((i) => i.name)).toContain('sections');

expect(inputs().map((i) => i.name)).not.toContain('layout');
expect(input('layout')).toBeUndefined();
});

it('no input description still teaches `layout` as an authorable key', () => {
// The declaration and its prose died together: `sections` used to end
// "Required when layout is 'custom'", which would have kept teaching the
// key from a neighbouring description after the input itself was gone.
const descriptions = inputs().map((i) => i.description ?? '');
expect(descriptions.join('')).not.toBe('');
for (const description of descriptions) {
expect(description).not.toMatch(/\blayout is\b/i);
expect(description).not.toMatch(/layout:\s*['"]?(auto|custom)/i);
}
});

it('the renderer contains no `schema.layout` read', () => {
// Source-text half. The behavioural twin lives in
// `recordDetailsBodySource.test.tsx` (authoring the key changes no output);
// this one catches a re-added branch that a render assertion could miss if
// the branch were re-introduced behind a condition the fixtures never hit.
const src = readFileSync(join(SRC_DIR, 'renderers/record-details.tsx'), 'utf8');
expect(src).toContain('RecordDetailsRenderer'); // the file really is the renderer
expect(src).not.toMatch(/schema\s*\.\s*layout/);
expect(src).not.toMatch(/['"]compact['"]/);
});
});
Loading
Loading