fix(plugin-chatbot): fence chatbot-floating's raw props spread

.changeset/7708-chatbot-floating-spread-fence.md

@@ -0,0 +1,34 @@
+---
+'@object-ui/types': minor
+'@object-ui/plugin-chatbot': minor
+---
+
+`chatbot-floating` now fences its `<FloatingChatbot>` spread the same way its
+two sibling registrations (`chatbot`, `chatbot-enhanced`) already do —
+`{...toDomProps(props)}`, at the head of the element, instead of a raw
+`{...props}` spread at the end (objectui#7708). This is a deliberate,
+user-visible behavior change, not a refactor:
+
+- **A message sent through a floating chatbot now actually renders.**
+  Previously the authored `messages` seed (whatever array was on the node
+  when it was authored) silently overrode the live runtime messages on every
+  render, because the raw spread landed AFTER `messages={runtimeMessages}`.
+  Neither the user's own message nor an `autoResponse` reply ever appeared —
+  the identical send on `chatbot-enhanced` worked correctly. Fixed.
+- **`displayMode`, `systemPrompt` and `model` stop leaking as DOM attributes**
+  on the panel's root element (`systemPrompt` / `model` are still read
+  normally, by name, for the request they configure — only the second,
+  unfiltered forward is gone). Closes objectui#4425's leak class on the one
+  `plugin-chatbot` registration that had not closed it yet.
+- **Three undeclared keys go dark on `chatbot-floating` nodes:**
+  `processVisibility`, `surface` and `showAvatars` reached the panel's
+  `ChatbotEnhanced` through the raw spread even though `ChatbotFloatingSchema`
+  never declared them. `ChatbotFloatingSchema` documents this explicitly and
+  always has — the face never promised these keys — so this closes an
+  accidental channel rather than removing declared behavior. A document that
+  relied on any of the three to affect a floating node loses that effect;
+  author them on a `chatbot-enhanced` node instead, where they are part of
+  the declared, tested contract.
+
+`@object-ui/types`: `ChatbotFloatingSchema`'s doc comment is updated to match
+— no type-shape change, so nothing that imports the type needs to change.

packages/plugin-chatbot/src/__tests__/renderer.authoring-faces-7655.test.tsx

@@ -35,19 +35,25 @@
  *      below measure the OUTCOME through the real SDUI host — the composer is
  *      disabled when the node says so and enabled when it does not — which is
  *      what must not move.
- *   4. **`chatbot-floating` has a second channel the named-read census cannot
- *      see — TRIPWIRE, not contract.** The registration ends its
- *      `FloatingChatbot` element with a raw `{...props}` spread, LAST, where
- *      its two siblings spread `toDomProps(props)` FIRST. So three keys
- *      `ChatbotFloatingSchema` does NOT declare — and the named-read census in
+ *   4. **`chatbot-floating` no longer has the second channel the named-read
+ *      census could not see — CLOSED, objectui#7708.** The registration used to
+ *      end its `FloatingChatbot` element with a raw `{...props}` spread, LAST,
+ *      where its two siblings spread `toDomProps(props)` FIRST — so three keys
+ *      `ChatbotFloatingSchema` does NOT declare, and the named-read census in
  *      `@object-ui/types`' `chatbot-registration-authoring-faces-7655.test.ts`
- *      correctly reads 0 for — still reach the panel's `ChatbotEnhanced`:
- *      `showAvatars`, `surface`, `processVisibility`. The cases below pin that
- *      MEASUREMENT (lit/dark pairs on a floating node, `chatbot-enhanced` as the
- *      control) so the face's docblock cannot rot silently. They do not make the
- *      channel a contract: fencing the spread like the siblings, or declaring
- *      the keys, is objectui#7708's ruling, and whichever lands flips these
- *      pins with it — deliberately, never silently.
+ *      correctly read 0 for, still reached the panel's `ChatbotEnhanced`:
+ *      `showAvatars`, `surface`, `processVisibility`. objectui#7708 ruled
+ *      "fence like the siblings" (triage comment 5550678895) over "declare the
+ *      three keys" — fencing also closes that card's p2 half (an authored
+ *      `messages` seed overriding the live runtime messages) and the DOM-
+ *      attribute leak, neither of which a declare-only fix would have touched.
+ *      The cases below WERE a lit/dark MEASUREMENT of the open channel; they now
+ *      pin its closure — dark on `chatbot-floating` regardless of the key,
+ *      `chatbot-enhanced` unchanged as the control (it reads these by name).
+ *      Flipped deliberately with the fix, never silently — see
+ *      `renderer.floating-spread-fence-7708.test.tsx` for the fix's own new
+ *      coverage (the `messages` override and the attribute leak, neither of
+ *      which was pinned anywhere before this card).
  *
  * The runtime cases render through `SchemaRenderer`, not the bare component,
  * for the reason `renderer.surface.test.tsx` gives: what is measured is what
@@ -182,7 +188,7 @@ describe('chatbot-floating: `disabled` is the host-evaluated verdict (objectui#7
   });
 });
 
-/* ── 4. The raw spread is a second channel — TRIPWIRE for objectui#7708 ── */
+/* ── 4. The raw spread WAS a second channel — CLOSED by objectui#7708 ── */
 
 /** An assistant turn with a tool result: `processVisibility: 'debug'` shows the raw tool name; `'summary'` (the default) does not. */
 const ASSISTANT_WITH_TOOL = [
@@ -222,35 +228,37 @@ async function renderNode(kind: 'chatbot-floating' | 'chatbot-enhanced', extra:
 /** The per-message avatar `MessageAvatar` renders only when `showAvatars` is on. */
 const AVATAR = 'div.size-7.rounded-full[aria-hidden="true"]';
 
-describe('chatbot-floating: three undeclared keys are LIVE through the raw spread — tripwire for objectui#7708', () => {
+describe('chatbot-floating: three undeclared keys are DARK now the spread is fenced — objectui#7708', () => {
   it('the spread really is the difference: `showAvatars` survives `props` and not `toDomProps(props)`', () => {
     // The mechanism, pinned on the whitelist itself so the render readings
-    // below have a stated cause and not just a correlation.
+    // below have a stated cause and not just a correlation. Unchanged by the
+    // fix — this pins `toDomProps` itself, not which spread the registration
+    // now uses.
     const props = { showAvatars: true, surface: 'plain', processVisibility: 'debug', className: 'x' };
     expect(toDomProps(props)).not.toHaveProperty('showAvatars');
     expect(toDomProps(props)).not.toHaveProperty('surface');
     expect(toDomProps(props)).not.toHaveProperty('processVisibility');
     expect(toDomProps(props)).toHaveProperty('className', 'x'); // lit control
   });
 
-  it('`showAvatars: true` renders the message avatar on a floating node (lit 1 / dark 0); dark on `chatbot-enhanced` either way', async () => {
-    expect((await renderNode('chatbot-floating', { showAvatars: true })).querySelectorAll(AVATAR)).toHaveLength(1);
+  it('`showAvatars: true` no longer renders the message avatar on a floating node (dark either way); still dark on `chatbot-enhanced`, which never named-reads it', async () => {
+    expect((await renderNode('chatbot-floating', { showAvatars: true })).querySelectorAll(AVATAR)).toHaveLength(0);
     cleanup();
     expect((await renderNode('chatbot-floating', {})).querySelectorAll(AVATAR)).toHaveLength(0);
     cleanup();
     expect((await renderNode('chatbot-enhanced', { showAvatars: true })).querySelectorAll(AVATAR)).toHaveLength(0);
   });
 
-  it("`surface: 'plain'` reaches the floating panel (two `.max-w-2xl` wrappers lit / 0 dark) — `chatbot-enhanced` reads it by name, so it lights there too", async () => {
-    expect((await renderNode('chatbot-floating', { surface: 'plain' })).querySelectorAll('.max-w-2xl')).toHaveLength(2);
+  it("`surface: 'plain'` no longer reaches the floating panel (dark either way) — `chatbot-enhanced` still reads it by name, so it lights there unchanged", async () => {
+    expect((await renderNode('chatbot-floating', { surface: 'plain' })).querySelectorAll('.max-w-2xl')).toHaveLength(0);
     cleanup();
     expect((await renderNode('chatbot-floating', {})).querySelectorAll('.max-w-2xl')).toHaveLength(0);
     cleanup();
     expect((await renderNode('chatbot-enhanced', { surface: 'plain' })).querySelectorAll('.max-w-2xl')).toHaveLength(2);
   });
 
-  it("`processVisibility: 'debug'` reaches the floating panel (raw tool name shown / hidden at the default) — named read lights `chatbot-enhanced` the same way", async () => {
-    expect((await renderNode('chatbot-floating', { processVisibility: 'debug' })).textContent).toContain('search_records');
+  it("`processVisibility: 'debug'` no longer reaches the floating panel (raw tool name stays hidden either way) — named read still lights `chatbot-enhanced` unchanged", async () => {
+    expect((await renderNode('chatbot-floating', { processVisibility: 'debug' })).textContent).not.toContain('search_records');
     cleanup();
     expect((await renderNode('chatbot-floating', {})).textContent).not.toContain('search_records');
     cleanup();

packages/plugin-chatbot/src/__tests__/renderer.floating-spread-fence-7708.test.tsx

@@ -0,0 +1,171 @@
+/**
+ * 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.
+ */
+
+/**
+ * objectui#7708 — the fix, not the tripwire.
+ *
+ * `chatbot-floating`'s registration ended its `<FloatingChatbot>` element with
+ * a raw `{...props}` spread, LAST, where its two siblings (`chatbot`,
+ * `chatbot-enhanced`) spread `{...toDomProps(props)}` FIRST. The card ruled
+ * "fence" (comment 5550678895 on the card, PM dispatch): move the spread to
+ * the head and filter it through `toDomProps`, matching the siblings.
+ *
+ * Three consequences followed from the unfenced spread; this file owns the
+ * two that were NOT already pinned as a measurement:
+ *
+ *   - The authored `messages` seed overrode the LIVE `messages={runtimeMessages}`
+ *     prop written above it, so a message the user actually sent on a floating
+ *     node never rendered — neither the user's own bubble nor the auto-reply.
+ *     This is the p2 half of the card's grade (triage comment 5550678895):
+ *     user-visible, on every floating chatbot. HEADLINE test below.
+ *   - Node keys the panel does not consume by name (`displayMode`,
+ *     `systemPrompt`, `model`) reached the panel root as DOM attributes
+ *     (objectui#4425's leak class) — even though `systemPrompt` / `model` ARE
+ *     legitimately read, by `useObjectChat`, off `schema` directly. The spread
+ *     forwarded them a SECOND time, raw, as unrelated top-level props.
+ *
+ * The third consequence — `showAvatars` / `surface` / `processVisibility`
+ * going dark on `chatbot-floating` — was already measured as a tripwire in
+ * `renderer.authoring-faces-7655.test.tsx` section 4; this card flips those
+ * pins from lit to dark rather than re-measuring them here.
+ *
+ * Pinning a bug as expected behaviour is the wrong shape (see this file's own
+ * name): these two cases did not exist anywhere as a pin before this fix, so
+ * the fix owns them fresh, as regression tests for the FIXED outcome.
+ */
+
+import '@testing-library/jest-dom/vitest';
+import { describe, it, expect, beforeAll, afterEach } from 'vitest';
+import { render, waitFor, cleanup } from '@testing-library/react';
+import { SchemaRenderer, SchemaRendererProvider } from '@object-ui/react';
+// Side-effect import: this is what registers the chat components.
+import '../renderer';
+
+const FAKE_ADAPTER = {
+  find: async () => [],
+  findOne: async () => null,
+  aggregate: async () => [],
+  count: async () => 0,
+  getObject: async () => null,
+};
+
+beforeAll(() => {
+  // `use-stick-to-bottom` (the enhanced composer's scroller) measures through
+  // ResizeObserver, which happy-dom does not implement.
+  (globalThis as Record<string, unknown>).ResizeObserver ??= class {
+    observe() {}
+    unobserve() {}
+    disconnect() {}
+  };
+  if (typeof Element !== 'undefined' && !Element.prototype.scrollIntoView) {
+    (Element.prototype as unknown as { scrollIntoView: () => void }).scrollIntoView = () => {};
+  }
+});
+
+afterEach(() => {
+  cleanup();
+  // The floating chatbot's portal container is appended to `document.body`
+  // and is NOT owned by RTL, so `cleanup()` does not take it with it — left
+  // behind, it would make the next test's body scan see this test's markup
+  // (same trap `widget-dom-leak-sweep.test.tsx` documents).
+  for (const node of Array.from(document.querySelectorAll('#floating-chatbot-portal'))) {
+    node.remove();
+  }
+});
+
+/** Renders a `chatbot-floating` node with the panel open through the real SDUI host. */
+async function renderFloating(extra: Record<string, unknown>): Promise<HTMLTextAreaElement> {
+  render(
+    <SchemaRendererProvider dataSource={FAKE_ADAPTER as never}>
+      <SchemaRenderer
+        schema={{
+          type: 'plugin-chatbot:chatbot-floating',
+          id: 'chat-node',
+          floatingConfig: { defaultOpen: true, title: 'Chat' },
+          ...extra,
+        } as never}
+        dataSource={FAKE_ADAPTER as never}
+      />
+    </SchemaRendererProvider>,
+  );
+  // The panel mounts through a portal onto `document.body`, so query there.
+  await waitFor(() => {
+    if (!document.body.querySelector('textarea')) {
+      throw new Error(`the floating panel never reached its composer. Body was:\n${document.body.innerHTML.slice(0, 600)}`);
+    }
+  });
+  return document.body.querySelector('textarea') as HTMLTextAreaElement;
+}
+
+describe('chatbot-floating: a sent message renders on the panel (fixed, objectui#7708)', () => {
+  it('the authored `messages` seed no longer overrides the live runtime messages', async () => {
+    const composer = await renderFloating({
+      messages: [{ id: 'seed', role: 'assistant', content: 'seed-message-marker' }],
+      autoResponse: true,
+      autoResponseText: 'auto-reply-marker',
+      autoResponseDelay: 0,
+    });
+
+    // Before the fix: the raw `{...props}` spread, LAST, put the AUTHORED
+    // `messages` array (the one-element seed above) back onto the panel after
+    // `messages={runtimeMessages}` had already written the live array — so
+    // nothing sent through the composer ever appeared, no matter how long the
+    // test waited. Fenced through `toDomProps`, `messages` is no longer in the
+    // forwarded set at all: the named `messages={runtimeMessages}` prop is the
+    // only carrier.
+    const form = composer.closest('form');
+    expect(form).not.toBeNull();
+    (composer as unknown as { value: string }).value = 'user-sent-marker';
+    composer.dispatchEvent(new Event('change', { bubbles: true }));
+    form!.dispatchEvent(new Event('submit', { bubbles: true, cancelable: true }));
+
+    // The user's own message renders immediately (no timer involved).
+    await waitFor(() => {
+      expect(document.body.textContent).toContain('user-sent-marker');
+    });
+    // The auto-reply renders once the (zero-delay) timer fires.
+    await waitFor(() => {
+      expect(document.body.textContent).toContain('auto-reply-marker');
+    });
+    // The seed message is untouched by the fix — it rendered before too (the
+    // card's own probe table: "seed shown" on both sides of the fence).
+    expect(document.body.textContent).toContain('seed-message-marker');
+  });
+});
+
+describe('chatbot-floating: node keys stop landing as DOM attributes on the panel (fixed, objectui#7708)', () => {
+  it('`systemPrompt` / `model` / `displayMode` no longer leak, though `systemPrompt` and `model` are still read by name', async () => {
+    await renderFloating({
+      systemPrompt: 'leak-canary-system-prompt',
+      model: 'leak-canary-model',
+      // `displayMode` is a retired `?: never` tombstone on the TS face
+      // (objectui#7654) but `BaseSchema` is `.passthrough()`, so an untyped
+      // JSON document can still carry it — the spread does not know the
+      // difference, so the probe stays honest about the raw channel.
+      displayMode: 'leak-canary-display-mode',
+    } as Record<string, unknown>);
+
+    const root = document.body.querySelector('[data-obj-id="chat-node"]');
+    expect(root, 'no element carries data-obj-id — the panel root was not found').not.toBeNull();
+
+    // The named half: `useObjectChat` still reads `systemPrompt` / `model` off
+    // `schema` directly (unaffected by fencing the SEPARATE top-level-prop
+    // channel) — this suite does not re-assert that plumbing, only that the
+    // leak is gone. The DOM half:
+    expect(root).not.toHaveAttribute('systemprompt');
+    expect(root).not.toHaveAttribute('model');
+    expect(root).not.toHaveAttribute('displaymode');
+
+    // The mechanism half — the stringified-object canary from
+    // `renderer.domProps.test.tsx`: nothing non-DOM survives under any name.
+    const stringified = Array.from((root as Element).attributes).filter((attribute) =>
+      attribute.value.includes('[object Object]'),
+    );
+    expect(stringified.map((attribute) => `${attribute.name}="${attribute.value}"`)).toEqual([]);
+  });
+});

packages/plugin-chatbot/src/renderer.tsx

@@ -433,6 +433,20 @@ ComponentRegistry.register('chatbot-floating',
 
     return (
       <FloatingChatbot
+        // Fenced and FIRST — matches the two sibling registrations above
+        // (objectui#7708). Was a raw `{...props}` spread LAST: every authored
+        // key `SchemaRenderer` forwarded reached the panel's `ChatbotEnhanced`
+        // unfiltered, so `processVisibility`, `surface` and `showAvatars` were
+        // live here although `ChatbotFloatingSchema` declares none of them,
+        // AND the authored `messages` seed overrode the runtime `messages`
+        // prop written below — a sent message never rendered on a floating
+        // node. Filtering through `toDomProps` and moving it first closes
+        // both: only the DOM-safe whitelist (plus `id`/`data-*`/`aria-*`)
+        // survives, and every named prop below now wins over it. No member
+        // `ChatbotFloatingSchema` declares depends on this channel — each is
+        // consumed by `useObjectChat` above or forwarded by name below, so
+        // fencing dark-outs nothing the face promises.
+        {...toDomProps(props)}
         floatingConfig={schema.floatingConfig}
         messages={runtimeMessages}
         placeholder={schema.placeholder}
@@ -452,16 +466,6 @@ ComponentRegistry.register('chatbot-floating',
         enableMarkdown={schema.enableMarkdown ?? true}
         enableFileUpload={schema.enableFileUpload ?? false}
         className={className}
-        // ⚠️ Raw and LAST — the two sibling registrations spread
-        // `toDomProps(props)` FIRST. Every authored key `SchemaRenderer`
-        // forwards reaches the panel's `ChatbotEnhanced` unfiltered (so
-        // `processVisibility`, `surface` and `showAvatars` are live here
-        // although `ChatbotFloatingSchema` declares none of them), and the
-        // authored `messages` seed overrides the runtime `messages` prop
-        // written above. Measured through the real host and carded as objectui#7708
-        // — fence vs declare is that card's ruling. Deliberately NOT changed
-        // by objectui#7655, which declared faces and moved no render outcome.
-        {...props}
       />
     );
   },