From 6add622d2b18388d91fbe69f390c9bb9bfef6aa3 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 6 Sep 2026 14:48:52 +0000 Subject: [PATCH] fix(plugin-chatbot): fence chatbot-floating's raw props spread (#7708) chatbot-floating ended its element with a raw {...props} spread, LAST, where its two sibling registrations (chatbot, chatbot-enhanced) spread {...toDomProps(props)} FIRST. Per the card's ruling (fence, not declare), moving it to the head and filtering it through toDomProps closes all three consequences at once: a sent message now renders on a floating chatbot (the authored `messages` seed no longer overrides the live runtime messages), processVisibility / surface / showAvatars go dark on chatbot-floating nodes (matching what ChatbotFloatingSchema has always declared), and displayMode / systemPrompt / model stop leaking as DOM attributes on the panel root. The three tripwire pins in renderer.authoring-faces-7655.test.tsx (section 4) flip from lit to dark, deliberately, as planned when they were written. New coverage owns the two consequences that were not pinned anywhere before this fix: renderer.floating-spread-fence-7708.test.tsx. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YBWFb5YgMU5dw8p2VKj16S --- .../7708-chatbot-floating-spread-fence.md | 34 ++++ .../renderer.authoring-faces-7655.test.tsx | 50 ++--- ...nderer.floating-spread-fence-7708.test.tsx | 171 ++++++++++++++++++ packages/plugin-chatbot/src/renderer.tsx | 24 ++- packages/types/src/complex.ts | 27 +-- 5 files changed, 262 insertions(+), 44 deletions(-) create mode 100644 .changeset/7708-chatbot-floating-spread-fence.md create mode 100644 packages/plugin-chatbot/src/__tests__/renderer.floating-spread-fence-7708.test.tsx diff --git a/.changeset/7708-chatbot-floating-spread-fence.md b/.changeset/7708-chatbot-floating-spread-fence.md new file mode 100644 index 0000000000..3ad24f5747 --- /dev/null +++ b/.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 `` 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. diff --git a/packages/plugin-chatbot/src/__tests__/renderer.authoring-faces-7655.test.tsx b/packages/plugin-chatbot/src/__tests__/renderer.authoring-faces-7655.test.tsx index 3145758e29..bae3833bbc 100644 --- a/packages/plugin-chatbot/src/__tests__/renderer.authoring-faces-7655.test.tsx +++ b/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,10 +228,12 @@ 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'); @@ -233,24 +241,24 @@ describe('chatbot-floating: three undeclared keys are LIVE through the raw sprea 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(); diff --git a/packages/plugin-chatbot/src/__tests__/renderer.floating-spread-fence-7708.test.tsx b/packages/plugin-chatbot/src/__tests__/renderer.floating-spread-fence-7708.test.tsx new file mode 100644 index 0000000000..ad933f5472 --- /dev/null +++ b/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 `` 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).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): Promise { + render( + + + , + ); + // 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); + + 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([]); + }); +}); diff --git a/packages/plugin-chatbot/src/renderer.tsx b/packages/plugin-chatbot/src/renderer.tsx index d81a8f2b0e..1c2ec65385 100644 --- a/packages/plugin-chatbot/src/renderer.tsx +++ b/packages/plugin-chatbot/src/renderer.tsx @@ -433,6 +433,20 @@ ComponentRegistry.register('chatbot-floating', return ( ); }, diff --git a/packages/types/src/complex.ts b/packages/types/src/complex.ts index 78de4cd0ba..f920088597 100644 --- a/packages/types/src/complex.ts +++ b/packages/types/src/complex.ts @@ -1240,8 +1240,9 @@ export interface ChatbotSchema extends BaseSchema { * NAMED `schema.KEY` read in each of the three `ComponentRegistry.register(...)` * bodies of `packages/plugin-chatbot/src/renderer.tsx`, forwarded into * `useObjectChat` or onto the rendered component. (Named reads are the - * instrument; the `chatbot-floating` registration also has an unfiltered - * props spread — see {@link ChatbotFloatingSchema}.) The instrument was lit by + * instrument; the `chatbot-floating` registration also HAD an unfiltered + * props spread on this census's base — fenced since, objectui#7708; see + * {@link ChatbotFloatingSchema}.) The instrument was lit by * keys that are NOT shared — `processVisibility` read 0 / 1 / 0 across * `chatbot` / `chatbot-enhanced` / `chatbot-floating` and `floatingConfig` * 0 / 0 / 1 — so a zero in that census is a reading, not a blind grep. @@ -1374,17 +1375,17 @@ export interface ChatbotEnhancedSchema * members no registration reads by name. `disabled` / `className` are * inherited from {@link BaseSchema}, as on the two sibling faces. * - * ⚠️ The named-read census is not the only channel. This registration ends - * its `` element with a raw `{...props}` spread — every - * authored key `SchemaRenderer` forwards, unfiltered — and the panel is a - * ``, so an authored `processVisibility`, `surface` or - * `showAvatars` DOES reach it today (measured through the real host: each - * lights its marker on a `chatbot-floating` node and stays dark without the - * key, while `chatbot-enhanced`, whose spread is `toDomProps`-filtered, keeps - * `showAvatars` dark). That channel is accidental, not contract: declaring - * the three here would fossilise it (AGENTS.md #0.1), and fencing it is a - * behaviour change with its own review. Recorded on its own card, - * objectui#7708; this face neither declares nor promises it. + * The named-read census used to not be the only channel: this registration + * ended its `` element with a raw `{...props}` spread — + * every authored key `SchemaRenderer` forwards, unfiltered — and the panel + * is a ``, so an authored `processVisibility`, `surface` or + * `showAvatars` reached it (measured through the real host, objectui#7708). + * That channel was accidental, not contract, and is now CLOSED: the spread + * is fenced through `toDomProps` and moved to the head of the element, the + * same shape the two sibling registrations already use, so this face's + * declared set is now also its delivered set. `processVisibility` and + * `surface` are dark on `chatbot-floating` on purpose, same as `showAvatars` + * above — author them on `chatbot-enhanced` instead. */ export interface ChatbotFloatingSchema extends BaseSchema,