refactor(plugin-chatbot): useObjectChat declares the message shape it actually returns (#4424)

.changeset/useobjectchat-honest-message-type-4424.md

@@ -0,0 +1,16 @@
+---
+'@object-ui/plugin-chatbot': minor
+'@object-ui/app-shell': patch
+---
+
+`useObjectChat` declares the message shape it actually hands back
+
+The hook typed `messages` — and the `onSend(content, messages)` callback fed from it — as `@object-ui/types`' authoring `ChatMessage`. That was true in local mode only. In API mode the values came out of the runtime mapper and were asserted into place with `as OuiChatMessage[]`, and the authoring contract declares none of what they carry: `buildProgress`, `blueprintProgress`, `charts`, and `pendingActionId` / `draftReview` / `proposedPlan` / `proposedChanges` / `builderHandoff` on every tool invocation. Those keys are the HITL approval card, the "Review N changes" affordance, the proposed-plan card, the build panel and the inline charts. They survived only because nothing on the path ever rebuilt a message; anyone writing the obvious thing — reconstruct a message field-by-field from its declared type — deleted all of them, with the compiler agreeing, because the declared type genuinely did not have them.
+
+The declaration is now the truth, published as `ObjectChatMessage`. The survey behind it found the honest type to be neither of the two `ChatMessage` types on either side, because neither is true of both modes: it stays **wide** where local mode is wide (an authored `'tool'` role and the legacy `'partial-call'` / `'call'` / `'result'` tool states reach this surface unchanged and are folded only at the render seam), **narrow** where both modes are narrow (`timestamp` is `string`, never `Date` — API mode never produces one and local mode absorbs it before emitting), and adds the render-only keys API mode really carries. The `as OuiChatMessage[]` assertion is deleted rather than moved: the mapper's output satisfies the declared type, so the compiler checks that assignment instead of being told to stop looking.
+
+Nothing about the values changed, and nothing correct breaks. `ObjectChatMessage` is a **subtype** of the authoring `ChatMessage` it replaces, so every consumer that accepted the old declaration still accepts these values — including a host `onSend` callback that types its parameter as `ChatMessage[]`, which keeps type-checking by contravariance. Naming `ObjectChatMessage` is what lets a host *read* the keys above. The one observable narrowing is deliberate: code that branched on `timestamp instanceof Date` was handling a value this hook cannot emit, and now says so at compile time.
+
+The seam below it (`chatMessageAdapter.ts`, from objectui#4399) is still necessary and unchanged in behaviour — `'tool'` and the legacy tool states still have to be narrowed for the renderers. What changed is that its pass-through is no longer an act of faith: its input type (`SeamChatMessage`, also exported, alongside `SeamToolInvocation`) names the render-only keys, so the spread preserves them as declared properties the compiler can see, and the pass-through tests type their API-mode fixture directly instead of casting it past the compiler. A cast returning to the hook is now caught by a test rather than by a future outage.
+
+App-shell carries a comment-only correction on the same family: `AiChatPage` still described `@object-ui/plugin-chatbot` as exporting a second, minimal legacy `ChatMessage` alongside the enhanced one. That collision was retired in objectui#4383 — the barrel publishes one contract and `ChatbotEnhancedMessage` is a deprecated alias of it — so the paragraph was sending readers to look for a hazard that no longer exists.

content/docs/plugins/plugin-chatbot.mdx

@@ -98,7 +98,7 @@ const schema = {
   autoResponse?: boolean,
   autoResponseText?: string,
   autoResponseDelay?: number,
-  onSend?: (content: string, messages: ChatMessage[]) => void,
+  onSend?: (content: string, messages: ObjectChatMessage[]) => void,
   className?: string,
   // AI / service-ai integration fields
   api?: string,
@@ -494,6 +494,29 @@ const aiSchema: ChatbotSchema = {
 }
 ```
 
+### What comes back out: `ObjectChatMessage`
+
+You AUTHOR with `@object-ui/types`' `ChatMessage`. What `useObjectChat` HANDS
+BACK — from `messages` and from `onSend(content, messages)` — is
+`ObjectChatMessage`, exported from `@object-ui/plugin-chatbot`:
+
+```plaintext
+import type { ObjectChatMessage } from '@object-ui/plugin-chatbot'
+```
+
+It is the authoring shape plus the render-only keys API mode really carries
+(`buildProgress`, `blueprintProgress`, `charts`, and `pendingActionId` /
+`draftReview` / `proposedPlan` / `proposedChanges` / `builderHandoff` on each
+tool invocation — the approval card, the "Review N changes" affordance, the plan
+card, the build panel, the inline charts), with `timestamp` narrowed to `string`
+because both modes absorb an authored `Date` before emitting.
+
+It is a subtype of the authoring `ChatMessage`, so an `onSend` callback that
+already declares `ChatMessage[]` keeps type-checking; naming `ObjectChatMessage`
+is what lets it read those keys. Rebuilding a message field-by-field from the
+authoring type drops every one of them — silently, and with the compiler's
+agreement (objectui#4424).
+
 ## Related Documentation
 
 - [Plugin System Overview](/docs/guide/plugins)

packages/app-shell/src/console/ai/AiChatPage.tsx

@@ -77,14 +77,17 @@ import {
   type ChatbotEnhancedToolInvocation,
   // The ENHANCED message shape — the one `<ChatbotEnhanced>` renders and the
   // one this file actually produces (`toolInvocations`, `buildProgress`).
-  // `@object-ui/plugin-chatbot` ALSO exports a minimal legacy `ChatMessage`
-  // from its own barrel module (id/role/content/timestamp/avatar only), and
-  // that is what this import used to resolve to. The mismatch compiled because
-  // every construction site spreads the extra keys conditionally
-  // (`...(x ? { toolInvocations } : {})`), which defeats excess-property
-  // checking — so the declared type was narrower than every value flowing
-  // through it, and `AiChatPage.hydration.test.ts` could not read
-  // `toolInvocations` off its own function's return (objectui#4040).
+  //
+  // `@object-ui/plugin-chatbot` publishes ONE chat-message contract: its
+  // barrel's `ChatMessage` IS this type, and `ChatbotEnhancedMessage` is a
+  // deprecated alias of the same declaration, kept so this import (PR #4379)
+  // keeps compiling. The collision that made the alias necessary is gone —
+  // the barrel used to DECLARE a second, minimal `ChatMessage` of its own
+  // (id/role/content/timestamp/avatar only) and the natural name resolved to
+  // it, which is how this file once could not read `toolInvocations` off its
+  // own function's return (objectui#4040). Retired in objectui#4383 / PR
+  // #4400; pinned in the plugin's `chat-message-contract.test.ts`. New code
+  // here should spell `ChatMessage`.
   type ChatbotEnhancedMessage as ChatMessage,
 } from '@object-ui/plugin-chatbot';
 

packages/plugin-chatbot/README.md

@@ -110,6 +110,30 @@ function MyChat() {
 }
 ```
 
+#### What `messages` actually contains
+
+`messages` — and the `onSend(content, messages)` callback fed from it — is
+typed `ObjectChatMessage[]`, exported from this package. It is deliberately
+neither of the two `ChatMessage` types nearby, because neither is true of both
+modes (objectui#4424):
+
+- **Not `@object-ui/types`' authoring `ChatMessage`.** In API mode the values
+  come from the runtime mapper and carry `buildProgress`, `blueprintProgress`,
+  `charts`, and `pendingActionId` / `draftReview` / `proposedPlan` /
+  `proposedChanges` / `builderHandoff` on each tool invocation — the approval
+  card, the "Review N changes" affordance, the plan card, the build panel and
+  the inline charts. The authoring contract declares none of them, so rebuilding
+  a message field-by-field from it deletes all of them, and the compiler agrees.
+- **Not this package's runtime `ChatMessage` either.** In local mode an authored
+  `'tool'` role and the legacy `'partial-call'` / `'call'` / `'result'` tool
+  states pass through untouched; they are folded only at the render seam
+  (`toRuntimeMessages`, `chatMessageAdapter.ts`).
+
+`ObjectChatMessage` is a subtype of the authoring `ChatMessage`, so anything
+already typed against that keeps compiling — naming `ObjectChatMessage` is what
+lets you *read* the keys above. `timestamp` is always a `string` here: both
+modes absorb an authored `Date` before emitting.
+
 ## Schema-Driven Usage
 
 ### Discovering Backend Agents

packages/plugin-chatbot/src/__tests__/chat-message-contract.test.ts

@@ -42,7 +42,17 @@ import type { ChatMessage as BarrelChatMessage, ChatbotEnhancedMessage } from '.
 import type { ChatMessage as EnhancedChatMessage } from '../ChatbotEnhanced';
 /** The OTHER side of the seam: the JSON/SDUI authoring contract. */
 import type { ChatMessage as AuthoredChatMessage } from '@object-ui/types';
-import type { authoredToRuntimeMessage, toRuntimeMessages } from '../chatMessageAdapter';
+import type {
+  authoredToRuntimeMessage,
+  toRuntimeMessages,
+  SeamChatMessage,
+} from '../chatMessageAdapter';
+/** The hook's own output contract (objectui#4424). */
+import type {
+  ObjectChatMessage,
+  UseObjectChatOptions,
+  UseObjectChatReturn,
+} from '../useObjectChat';
 
 type Assert<T extends true> = T;
 type IsAny<T> = 0 extends 1 & T ? true : false;
@@ -204,6 +214,145 @@ describe('the authoring ↔ runtime seam is an adapter, not a cast', () => {
   });
 });
 
+describe("useObjectChat's declared message type is honest about both modes", () => {
+  it('is pinned at compile time', () => {
+    // objectui#4424. The hook declared `messages: OuiChatMessage[]` (the
+    // AUTHORING contract) and produced it honestly in local mode only — API
+    // mode built RUNTIME messages and cast them with `as OuiChatMessage[]`,
+    // erasing `buildProgress`, `blueprintProgress`, `charts` and every HITL /
+    // draft-review extension on the tool invocations. The values survived
+    // because nothing on the path rebuilt a message; the declaration was the
+    // trap, and these pins are what keeps it closed.
+
+    // Probe hygiene first — an `any`/`unknown` here answers every question.
+    type _HookMsgNotAny = Assert<Equal<IsAny<ObjectChatMessage>, false>>;
+    type _HookMsgNotUnknown = Assert<Equal<IsUnknown<ObjectChatMessage>, false>>;
+    type _NoIndexSignature = Assert<Equal<HasIndexSignature<ObjectChatMessage>, false>>;
+
+    // 1. The hook's two published surfaces speak it — `messages` and the
+    //    `onSend(content, messages)` callback, which the survey measured as
+    //    carrying the SAME values (both modes hand it the thread they are
+    //    about to hold).
+    type _ReturnIsHonest = Assert<Equal<UseObjectChatReturn['messages'], ObjectChatMessage[]>>;
+    type _OnSendIsHonest = Assert<
+      Equal<
+        NonNullable<UseObjectChatOptions['onSend']>,
+        (content: string, messages: ObjectChatMessage[]) => void
+      >
+    >;
+
+    // 2. WIDE where local mode is wide. An authored `'tool'` role and the
+    //    legacy tool states reach this surface unchanged (`normalizeMessages`
+    //    narrows neither) and are folded only at the render seam. This is the
+    //    line that proves the runtime type would have been a LIE about local
+    //    mode — i.e. why the honest answer is neither of the two existing
+    //    contracts.
+    type _KeepsToolRole = Assert<
+      Equal<ObjectChatMessage['role'], 'user' | 'assistant' | 'system' | 'tool'>
+    >;
+    type _KeepsLegacyToolStates = Assert<
+      Equal<
+        Extract<
+          NonNullable<NonNullable<ObjectChatMessage['toolInvocations']>[number]['state']>,
+          'partial-call' | 'call' | 'result'
+        >,
+        'partial-call' | 'call' | 'result'
+      >
+    >;
+
+    // 3. NARROW where both modes are narrow. Neither mode can emit a `Date`:
+    //    API mode never produces one, and local mode absorbs it in
+    //    `normalizeMessages` before handing anything out. Declaring
+    //    `string | Date` asked every consumer to handle a value that cannot
+    //    arrive.
+    type _TimestampIsString = Assert<Equal<ObjectChatMessage['timestamp'], string | undefined>>;
+    type _TimestampRejectsDate = Assert<
+      Equal<Equal<ObjectChatMessage['timestamp'], AuthoredChatMessage['timestamp']>, false>
+    >;
+
+    // 4. …plus the render-only keys API mode really carries. Named one by one
+    //    so a future narrowing says WHICH capability it dropped — each of these
+    //    is a rendered affordance: the build panel, the design panel, inline
+    //    charts, the approval card, the "Review N changes" entry point, the
+    //    proposed-plan card, the confirm-changes card, "Open in Builder".
+    type _HasBuildProgress = Assert<Has<ObjectChatMessage, 'buildProgress'>>;
+    type _HasBlueprintProgress = Assert<Has<ObjectChatMessage, 'blueprintProgress'>>;
+    type _HasCharts = Assert<Has<ObjectChatMessage, 'charts'>>;
+    type HookToolInvocation = NonNullable<ObjectChatMessage['toolInvocations']>[number];
+    type _HasPendingActionId = Assert<Has<HookToolInvocation, 'pendingActionId'>>;
+    type _HasDraftReview = Assert<Has<HookToolInvocation, 'draftReview'>>;
+    type _HasProposedPlan = Assert<Has<HookToolInvocation, 'proposedPlan'>>;
+    type _HasProposedChanges = Assert<Has<HookToolInvocation, 'proposedChanges'>>;
+    type _HasBuilderHandoff = Assert<Has<HookToolInvocation, 'builderHandoff'>>;
+
+    // 5. The compatibility statement, and the reason this is a MINOR and not a
+    //    break: the honest type is a SUBTYPE of the authoring one it replaces.
+    //    Every consumer that correctly accepted `@object-ui/types`' ChatMessage
+    //    still accepts these values — including a host `onSend` typed against
+    //    the authoring contract, which type-checks by contravariance.
+    type _StillAnAuthoredMessage = Assert<
+      ObjectChatMessage extends AuthoredChatMessage ? true : false
+    >;
+
+    // 6. And the seam is still NECESSARY — this is not option 2 in disguise
+    //    (API mode made to produce the authoring type, i.e. the field-by-field
+    //    rebuild). The hook's shape is deliberately NOT assignable to the
+    //    runtime one: `'tool'` and the legacy states still have to be narrowed,
+    //    by `chatMessageAdapter.ts`, at the render seam.
+    type _AdapterStillNeeded = Assert<
+      Equal<ObjectChatMessage extends EnhancedChatMessage ? true : false, false>
+    >;
+    //    The other direction DOES hold, and that is what deleted the cast: the
+    //    runtime mapper's output is assignable to the hook's declared type, so
+    //    API mode assigns instead of asserting. It is also what keeps a host's
+    //    own narrowing cast to the runtime type legal.
+    type _RuntimeFlowsInWithoutACast = Assert<
+      EnhancedChatMessage extends ObjectChatMessage ? true : false
+    >;
+
+    expect(true).toBe(true);
+  });
+});
+
+describe("the seam's input contract names the keys it passes through", () => {
+  it('is pinned at compile time', () => {
+    // objectui#4424. PR #4416's adapter preserved the runtime-only keys with a
+    // spread over a parameter typed as the AUTHORING message — so the keys were
+    // preserved by faith: `tsc` could not see one of them, and the pass-through
+    // test had to cast its fixture into place. `SeamChatMessage` names them.
+    type _SeamNotAny = Assert<Equal<IsAny<SeamChatMessage>, false>>;
+    type _SeamHasBuildProgress = Assert<Has<SeamChatMessage, 'buildProgress'>>;
+    type _SeamHasBlueprintProgress = Assert<Has<SeamChatMessage, 'blueprintProgress'>>;
+    type _SeamHasCharts = Assert<Has<SeamChatMessage, 'charts'>>;
+    type SeamTool = NonNullable<SeamChatMessage['toolInvocations']>[number];
+    type _SeamToolHasPendingActionId = Assert<Has<SeamTool, 'pendingActionId'>>;
+    type _SeamToolHasDraftReview = Assert<Has<SeamTool, 'draftReview'>>;
+
+    // It is the seam's INPUT, so it stays wide enough for both callers: a host
+    // holding plain authored messages (the reason the adapter is exported from
+    // the barrel at all) and the hook's own output.
+    type _AuthoredStillAccepted = Assert<
+      AuthoredChatMessage extends SeamChatMessage ? true : false
+    >;
+    type _HookOutputAccepted = Assert<ObjectChatMessage extends SeamChatMessage ? true : false>;
+
+    // Which is why it keeps the `Date` the hook's output has already absorbed:
+    // this is where an authored `Date` still dies (`toRuntimeTimestamp`), and
+    // narrowing it here would make that documented decision unreachable.
+    type _SeamStillTakesADate = Assert<
+      Equal<SeamChatMessage['timestamp'], string | Date | undefined>
+    >;
+
+    // The adapter reads it: a signature that drifted back to the bare authoring
+    // type would re-blind the pass-through without changing a line of its body.
+    type _AdapterTakesTheSeamType = Assert<
+      Equal<Parameters<typeof authoredToRuntimeMessage>[0], SeamChatMessage>
+    >;
+
+    expect(true).toBe(true);
+  });
+});
+
 describe('the three renderer call sites go through the adapter', () => {
   // The runtime net over the block above, for the `pnpm test` lane that erases
   // type assertions. The failure it catches is a cast coming BACK: `as any` on
@@ -258,6 +407,31 @@ describe('the Date → ISO absorption is expressed once', () => {
         'copy here is the third dialect the card exists to prevent.',
     ).toBe(false);
   });
+
+  it('asserts API-mode messages into the authoring type no more', () => {
+    // objectui#4424, the runtime net over the compile-time pins above. The
+    // defect was ONE expression: `uiMessagesToChatMessages(...)` asserted into
+    // the authoring array type. A cast coming back here re-erases every
+    // render-only key without changing any other line — and `tsc` stays green,
+    // because a cast is exactly the instruction to stop checking.
+    //
+    // Comment lines are dropped first, and that is not incidental: the hook
+    // NAMES the deleted expression in the comment that explains why it is gone
+    // (as does this test), so a guard reading raw source would fire on the
+    // documentation of its own fix. Prose about a cast is not a cast.
+    const codeOnly = source
+      .split('\n')
+      .filter((line) => !/^\s*(\/\/|\/\*|\*)/.test(line))
+      .join('\n');
+    expect(
+      /as\s+OuiChatMessage\[\]/.test(codeOnly),
+      'useObjectChat.ts casts its API-mode messages into the @object-ui/types ' +
+        'authoring contract again. That cast erases `buildProgress`, ' +
+        '`blueprintProgress`, `charts` and the HITL / draft-review extensions on ' +
+        'every tool invocation (objectui#4424). The hook declares ' +
+        '`ObjectChatMessage`, which those values satisfy — assign, do not assert.',
+    ).toBe(false);
+  });
 });
 
 describe('the barrel no longer declares a message shape of its own', () => {