feat(app-shell)!: a record page shows a discussion panel only when it composes one (objectui#7298 half two)

.changeset/7298-discussion-explicit-composition.md

@@ -0,0 +1,59 @@
+---
+'@object-ui/app-shell': minor
+---
+
+A record page shows a discussion panel if and only if it composes one
+(objectui#7298).
+
+**⚠️ Behaviour change on a shipped default — migration below.** `RecordDetailView`
+used to append a discussion panel below any record page whose tree placed no
+`record:discussion` / `record:chatter` node, and the only documented way to
+decline it was `assignedPage.disableDiscussion = true`. That escape hatch could
+never be written: `PageSchema` is a `strictObject`, so the key is a hard parse
+error rather than a dropped one, and the renderer reached it through an
+`as any`. Meanwhile the appended panel was hard-coded open for writes (comment
+input, reactions, threading), so a deliberately read-only page — over a
+`protection: { lock: 'full' }` platform object, where the object-side switch is
+unreachable too — had no authorable way to say no.
+
+Maintainer ruling of 2026-09-12 (decision batch #120 item 5): *"a page is what
+its author composes … nothing is appended by default and then removed by a
+negative flag."* So the append is removed together with the unauthorable read —
+nothing reads an undeclared page key any more — rather than the negative flag
+being added to the protocol.
+
+**MIGRATION — pages that relied on the automatic panel add one
+`record:discussion` node.** Put it where you want the conversation to sit:
+
+```ts
+regions: [
+  {
+    name: 'main',
+    components: [
+      { type: 'page:header', properties: { title: '{name}' } },
+      { type: 'record:details' },
+      { type: 'record:discussion' },
+    ],
+  },
+]
+```
+
+There is **no transition window and no opt-in flag** — this behaviour is gone in
+this release. The node's own config is honoured as authored, so
+`{ type: 'record:discussion', properties: { feed: { showCommentInput: false } } }`
+is now the ordinary way to show a conversation without a composer.
+
+**Who is affected.** Only **authored full pages** that omit the node. Synthesized
+default pages and slotted pages are unchanged — `buildDefaultPageSchema`
+composes `record:discussion` itself, which is why the out-of-the-box record page
+still has its panel.
+
+**Precedence, now enforced rather than documented.** `enable.feeds: false` on the
+object stays the object's switch and outranks the page: an object with feeds off
+shows no discussion panel, declared or not. Previously that gate sat on the
+auto-append alone, so a declared (or synthesized) node rendered a panel on a
+feeds-off object over a feed the view deliberately never fetched; the composed
+tree is now pruned before it renders. Record pages for objects that never opted
+out are untouched — the tree is handed through by reference.
+
+Docs: `content/docs/guide/slotted-pages.md`.

content/docs/guide/slotted-pages.md

@@ -42,6 +42,51 @@ Each slot accepts a single component schema or an array (arrays are
 flattened in place). Each slot is a **full replacement at the slot
 boundary** — there is no deep-merge or JSON-Patch in v1.
 
+## The discussion panel is composed, never appended
+
+A record page shows a discussion panel **if and only if the page composes a
+`record:discussion` node** — or its `record:chatter` alias, which is the same
+renderer under a Salesforce-familiar name. There is no automatic panel, and
+therefore no negative flag to switch one off.
+
+- **Synthesized** and **slotted** pages compose the node for you (it is the
+  `discussion` slot in the table above), so the out-of-the-box record page is
+  unchanged.
+- **Full pages** (`kind: "full"`) author every region, so a full page places the
+  node itself — exactly like every other component it wants:
+
+<!-- doc-snippet: fragment — a metadata excerpt: one region's components[] is one fragment of a page schema, shown without the document that would contain it -->
+```ts
+regions: [
+  {
+    name: 'main',
+    components: [
+      { type: 'page:header', properties: { title: '{name}' } },
+      { type: 'record:details' },
+      { type: 'record:discussion' },   // ← the panel is here because you put it here
+    ],
+  },
+]
+```
+
+The node's own config is honoured as authored, so the panel a page composes is
+the panel it asked for: `{ type: 'record:discussion', properties: { feed: {
+showCommentInput: false } } }` renders the conversation without a composer.
+
+**Precedence — the object outranks the page.** `enable.feeds: false` on the
+object definition suppresses the discussion panel whether or not the page
+composes the node; the view also skips the `sys_comment` read for such an
+object, and the server rejects comment writes against it with
+`403 FEEDS_DISABLED`. `enable.feeds` is opt-**out**: absent means on.
+
+> **Upgrading (objectui#7298).** A record page used to get a discussion panel
+> appended below its content whenever its tree placed no discussion node, and
+> the only way out was an `assignedPage.disableDiscussion` flag that `PageSchema`
+> — a `strictObject` — refuses, so no author could ever write it. Both are gone.
+> If one of your **authored full pages** relied on that automatic panel, add one
+> `record:discussion` node where you want it. Synthesized and slotted pages need
+> no change.
+
 ## Example: customize only the header
 
 ```ts

content/docs/plugins/plugin-detail.mdx

@@ -334,10 +334,14 @@ decision as a defect is how a warning channel gets ignored:
   **Load more** rather than rendering whole.
 
   Two members of the shape are **not** read on this path and authoring them
-  does nothing here: `filterMode` and `enableMentions` (objectui#8968). And the
-  panel the host auto-appends when a page omits a discussion block does **not**
-  run this pipeline, so it still renders the feed unfiltered and whole
-  (objectui#8983).
+  does nothing here: `filterMode` and `enableMentions` (objectui#8968).
+
+  There is no third path left to disagree with: the host used to auto-append a
+  panel below any page that omitted a discussion block, and objectui#7298
+  retired that append — a record page shows a discussion panel if and only if it
+  composes one. Every panel a user sees is therefore an authored or synthesized
+  block running this pipeline. The object's `enable.feeds: false` still
+  outranks the page and suppresses the panel either way.
 </Callout>
 
 ### RecordActivityTimeline Config

packages/app-shell/src/utils/__tests__/pageSchemaIntrospect.test.ts

@@ -1,49 +1,82 @@
 import { describe, it, expect } from 'vitest';
-import { hasExplicitDiscussion, hasExplicitAttachments } from '../pageSchemaIntrospect';
+import { stripDiscussionNodes, hasExplicitAttachments } from '../pageSchemaIntrospect';
 
-describe('hasExplicitDiscussion', () => {
-  it('returns false for nullish and primitive inputs', () => {
-    expect(hasExplicitDiscussion(null)).toBe(false);
-    expect(hasExplicitDiscussion(undefined)).toBe(false);
-    expect(hasExplicitDiscussion('record:discussion')).toBe(false);
-    expect(hasExplicitDiscussion(42)).toBe(false);
-  });
+/**
+ * Every `type` string anywhere in a (non-cyclic) tree, in document order.
+ * Asserting on this rather than on a hand-written expected tree keeps the
+ * cases below readings about WHICH NODES SURVIVED — a strip that flattened or
+ * dropped a sibling would show up here, where a `not.toContain` on the
+ * serialized tree would not.
+ */
+function types(tree: unknown): string[] {
+  const out: string[] = [];
+  const walk = (node: any) => {
+    if (!node || typeof node !== 'object') return;
+    if (Array.isArray(node)) return node.forEach(walk);
+    if (typeof node.type === 'string') out.push(node.type);
+    for (const key of ['children', 'items', 'body', 'components', 'regions']) walk(node[key]);
+    if (node.properties) for (const key of ['children', 'items']) walk(node.properties[key]);
+  };
+  walk(tree);
+  return out;
+}
 
-  it('detects record:discussion at the root', () => {
-    expect(hasExplicitDiscussion({ type: 'record:discussion' })).toBe(true);
+/**
+ * objectui#7298 — `enable.feeds: false` is the OBJECT's switch and it outranks
+ * whatever the page composed. `RecordDetailView` hands the composed tree
+ * through this before rendering it, so a declared (or synthesized)
+ * `record:discussion` never reaches a feeds-off object.
+ */
+describe('stripDiscussionNodes', () => {
+  it('returns nullish and primitive inputs unchanged', () => {
+    expect(stripDiscussionNodes(null)).toBeNull();
+    expect(stripDiscussionNodes(undefined)).toBeUndefined();
+    // A bare string is not a node, so it is not a discussion node either.
+    expect(stripDiscussionNodes('record:discussion')).toBe('record:discussion');
+    expect(stripDiscussionNodes(42)).toBe(42);
   });
 
-  it('detects the record:chatter alias', () => {
-    expect(hasExplicitDiscussion({ type: 'record:chatter' })).toBe(true);
+  it('returns null when the ROOT itself is the discussion node', () => {
+    // No container to remove it from — the total-function corner.
+    expect(stripDiscussionNodes({ type: 'record:discussion' })).toBeNull();
+    expect(stripDiscussionNodes({ type: 'record:chatter' })).toBeNull();
   });
 
-  it('detects discussion nested inside children/items/body/components', () => {
+  it('removes discussion nested inside children/items/body/components', () => {
     const node = (key: string) => ({
       type: 'foo',
-      [key]: [{ type: 'record:discussion' }],
+      [key]: [{ type: 'record:details' }, { type: 'record:discussion' }],
     });
-    expect(hasExplicitDiscussion(node('children'))).toBe(true);
-    expect(hasExplicitDiscussion(node('items'))).toBe(true);
-    expect(hasExplicitDiscussion(node('body'))).toBe(true);
-    expect(hasExplicitDiscussion(node('components'))).toBe(true);
+    for (const key of ['children', 'items', 'body', 'components']) {
+      expect(types(stripDiscussionNodes(node(key)))).toEqual(['foo', 'record:details']);
+    }
+  });
+
+  it('removes the record:chatter alias too', () => {
+    const page = { type: 'foo', children: [{ type: 'record:chatter' }, { type: 'record:details' }] };
+    expect(types(stripDiscussionNodes(page))).toEqual(['foo', 'record:details']);
   });
 
-  it('detects discussion nested inside properties.children/items', () => {
+  it('removes discussion nested inside properties.children/items', () => {
     expect(
-      hasExplicitDiscussion({
-        type: 'foo',
-        properties: { children: [{ type: 'record:discussion' }] },
-      }),
-    ).toBe(true);
+      types(
+        stripDiscussionNodes({
+          type: 'foo',
+          properties: { children: [{ type: 'record:discussion' }, { type: 'record:details' }] },
+        }),
+      ),
+    ).toEqual(['foo', 'record:details']);
     expect(
-      hasExplicitDiscussion({
-        type: 'foo',
-        properties: { items: [{ type: 'record:chatter' }] },
-      }),
-    ).toBe(true);
+      types(
+        stripDiscussionNodes({
+          type: 'foo',
+          properties: { items: [{ type: 'record:chatter' }] },
+        }),
+      ),
+    ).toEqual(['foo']);
   });
 
-  it('detects discussion nested inside regions[].components[] (synth + full pages)', () => {
+  it('removes discussion from regions[].components[] (synth + full pages)', () => {
     // Mirrors buildDefaultPageSchema output shape.
     const synthPage = {
       type: 'record',
@@ -62,57 +95,80 @@ describe('hasExplicitDiscussion', () => {
         },
       ],
     };
-    expect(hasExplicitDiscussion(synthPage)).toBe(true);
+    const stripped = stripDiscussionNodes(synthPage) as any;
+    expect(types(stripped)).toEqual(['record', 'page:header', 'page:tabs', 'page:tab']);
+    // Everything else about the page survives, including the scalars that are
+    // not nodes at all.
+    expect(stripped.template).toBe('full-width');
+    expect(stripped.regions[0].name).toBe('main');
+    expect(stripped.regions[0].width).toBe('full');
   });
 
-  it('returns false when no discussion node exists anywhere in the tree', () => {
+  it('strips deep nesting (page:tabs > page:tab > record:discussion)', () => {
     const page = {
-      type: 'record',
       regions: [
         {
-          name: 'main',
           components: [
-            { type: 'page:header' },
             {
               type: 'page:tabs',
-              items: [
-                { type: 'page:tab', children: [{ type: 'record:details' }] },
-                { type: 'page:tab', children: [{ type: 'record:history' }] },
-              ],
+              items: [{ type: 'page:tab', children: [{ type: 'record:discussion' }] }],
             },
           ],
         },
       ],
     };
-    expect(hasExplicitDiscussion(page)).toBe(false);
+    expect(types(stripDiscussionNodes(page))).toEqual(['page:tabs', 'page:tab']);
   });
 
-  it('handles deep nesting (page:tabs > page:tab > record:discussion)', () => {
+  it('IDENTITY-PRESERVING — a tree with no discussion node comes back BY REFERENCE', () => {
+    // This is what keeps the feeds-ON path (every object that has not opted
+    // out) from handing a new page identity to the renderer on every render.
     const page = {
+      type: 'record',
       regions: [
         {
+          name: 'main',
           components: [
+            { type: 'page:header' },
             {
               type: 'page:tabs',
               items: [
-                {
-                  type: 'page:tab',
-                  children: [{ type: 'record:discussion' }],
-                },
+                { type: 'page:tab', children: [{ type: 'record:details' }] },
+                { type: 'page:tab', children: [{ type: 'record:history' }] },
               ],
             },
           ],
         },
       ],
     };
-    expect(hasExplicitDiscussion(page)).toBe(true);
+    expect(stripDiscussionNodes(page)).toBe(page);
+  });
+
+  it('copies only the path it had to change', () => {
+    const untouched = { type: 'page:header' };
+    const page = {
+      type: 'record',
+      regions: [{ name: 'main', components: [untouched, { type: 'record:discussion' }] }],
+    };
+    const stripped = stripDiscussionNodes(page) as any;
+    expect(stripped).not.toBe(page);
+    // The sibling that had nothing removed is the SAME object, not a clone.
+    expect(stripped.regions[0].components[0]).toBe(untouched);
+  });
+
+  it('prunes a SHARED subtree in every place it appears', () => {
+    // The walker memoizes by node, so a second occurrence must get the pruned
+    // result rather than the original it was handed on the way in.
+    const shared: any = { type: 'page:section', children: [{ type: 'record:discussion' }] };
+    const page = { type: 'record', regions: [{ components: [shared] }, { components: [shared] }] };
+    expect(types(stripDiscussionNodes(page))).toEqual(['record', 'page:section', 'page:section']);
   });
 
   it('does not loop forever on cyclic schemas', () => {
     const a: any = { type: 'page:section' };
     const b: any = { type: 'page:section', children: [a] };
     a.children = [b];
-    expect(hasExplicitDiscussion(a)).toBe(false);
+    expect(() => stripDiscussionNodes(a)).not.toThrow();
   });
 });