From 5e79286f479066eb2be42898cd441b0b2d81e93b Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 19 Sep 2026 11:27:01 +0000 Subject: [PATCH 1/2] fix(plugin-chatbot): build discriminated tool parts at the producer and delete the last cast on useChat `useObjectChat`'s API-mode builder declared its part array as `Array>` and pushed plain objects into it. That is not the chat runtime's part union, and the mismatch was absorbed by an `as any` on the `messages` option instead of being reported. Re-measured 2x2 on this branch, each leg mutated on disk with hash proof and restored by state: only the both-casts-removed leg is red, with one TS2322 in each of the two type-check programs. The builder now CONSTRUCTS each part, so the option is checked and the cast is gone. Per the ruling on this chain (director seat, decision batch #86, 2026-09-08, option A, contract-first) the fix lands at the producer, never as a wider cast at the consumer. - The three approval states are reachable: they require the runtime's `approval` envelope alongside them, `ChatToolInvocation` gained it in the first clause of this chain, and the builder now constructs those arms from it. - The legacy authoring states `partial-call` / `call` / `result` are folded onto the lifecycle arms they mean instead of passing through as states the round-trip reader refuses. - The dead `toolName` member is no longer written onto a `tool-*` part: only the dynamic-tool arm declares one, and the reader derives the name off `type`. - `UseObjectChatOptions.initialMessages`' optional `parts` narrows to the store's own part array. Breaking for an external host that passes pre-built parts; nothing in this repo sets it. An invocation claiming an approval state with no envelope to back it is not constructible, and no envelope is invented for it: the state is derived from the data it does carry and the producer is told once. An ObjectStack HITL approval is deliberately not reported, since it is carried by `pendingActionId` and the mapper re-promotes the state from the tool result. Also lifts the `approval` envelope in the mapper's tool-invocation extraction, closing the disagreement where the hydrated path carried the envelope and the live path dropped it. The lift lands in the same round as its first reader. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq --- .changeset/8426-chat-parts-discriminated.md | 53 +++ .../mapMessages.approvalLift-8426.test.ts | 120 +++++++ ...bjectChat.discriminatedParts-8426.test.tsx | 316 ++++++++++++++++++ packages/plugin-chatbot/src/mapMessages.ts | 47 +++ packages/plugin-chatbot/src/useObjectChat.ts | 263 +++++++++++++-- 5 files changed, 776 insertions(+), 23 deletions(-) create mode 100644 .changeset/8426-chat-parts-discriminated.md create mode 100644 packages/plugin-chatbot/src/__tests__/mapMessages.approvalLift-8426.test.ts create mode 100644 packages/plugin-chatbot/src/__tests__/useObjectChat.discriminatedParts-8426.test.tsx diff --git a/.changeset/8426-chat-parts-discriminated.md b/.changeset/8426-chat-parts-discriminated.md new file mode 100644 index 0000000000..7e67735348 --- /dev/null +++ b/.changeset/8426-chat-parts-discriminated.md @@ -0,0 +1,53 @@ +--- +'@object-ui/plugin-chatbot': minor +--- + +Build the chat runtime's discriminated tool parts at the PRODUCER, and delete the last +`as any` on the `useChat` call (objectui#8426; director seat, decision batch #86, +2026-09-08, option A — contract-first). + +`useObjectChat`'s API-mode builder declared its part array as +`Array>` and pushed plain objects into it. That is not the +store's part union, and the mismatch was absorbed by a cast on the `messages` option +rather than reported. Measured 2x2 on this branch, each leg mutated on disk with hash +proof: with both that cast and the blanket one objectui#8378 removed gone, both +type-check programs turn red with one `TS2322` each — `Record` is not +assignable to the store's part type. The builder now CONSTRUCTS each part, so the +option is checked and the cast is gone. + +**Breaking, deliberately — `UseObjectChatOptions.initialMessages`.** A message's +optional `parts` member is now declared as the store's own part array instead of +`Array>`. Nothing in this repository sets it (the schema +renderer passes `schema.messages`; app-shell passes the output of +`hydratedMessagesToChatMessages`, whose literal declares no `parts`), so this is +visible only to a host that hands `useObjectChat` pre-built parts — which is exactly +the population the cast was hiding the mismatch from. Per this repo's version policy a +breaking change ships as `minor`; migration is to build real parts (or drop `parts` and +let the builder synthesize them from `content` / `toolInvocations`). + +Three behaviour changes ride with it, each measured rather than assumed: + +- **The three approval states are now reachable.** `approval-requested`, + `approval-responded` and `output-denied` require the runtime's `approval` envelope + alongside them; `ChatToolInvocation` gained that envelope in objectui#9229, and the + builder now constructs those arms from it. Before this, such an invocation was + emitted as an untyped object the store could not hold. +- **The legacy authoring states are folded, not passed through.** `partial-call`, + `call` and `result` are not runtime states; passing them through left the round-trip + reader refusing them, so the invocation came back with no state at all. They now fold + onto `input-streaming` / `input-available` / `output-available`. +- **The dead `toolName` member is no longer written onto a `tool-*` part.** Only the + dynamic-tool arm declares one, and the round-trip reader derives the name off the + part's `type`, so dropping it is behaviour-preserving. + +An invocation that claims an approval state with no envelope to back it is not +constructible, and no envelope is invented for it: the state is derived from the data +the invocation does carry and the producer is told once, by name. An ObjectStack HITL +approval (`pendingActionId` plus a `pending_approval` result) is deliberately NOT +reported — it is carried by that id, and the mapper re-promotes the state from the +result on the way back out. + +Also lifts the `approval` envelope in `mapMessages`' tool-invocation extraction, which +closes the disagreement objectui#9229 left behind: the hydrated path carried the +envelope while the live path dropped it. The lift lands in the same round as its first +reader, rather than earlier as a declared-but-unread key. diff --git a/packages/plugin-chatbot/src/__tests__/mapMessages.approvalLift-8426.test.ts b/packages/plugin-chatbot/src/__tests__/mapMessages.approvalLift-8426.test.ts new file mode 100644 index 0000000000..b0adef0ecf --- /dev/null +++ b/packages/plugin-chatbot/src/__tests__/mapMessages.approvalLift-8426.test.ts @@ -0,0 +1,120 @@ +/** + * 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. + */ + +/** + * `extractToolInvocations` lifts the chat runtime's `approval` envelope + * (objectui#8426, the clause assigned to this card when objectui#8442 shipped + * as the additive half). + * + * ## Why the lift had to wait for a reader + * + * The envelope had NO reader until the parts builder in `useObjectChat.ts` + * started constructing the discriminated arms, and a declared-but-unread key on + * a chat surface is the defect class this repo keeps carding. So the lift lands + * in the same round as its read site — and until it did, the two paths into the + * same conversation disagreed: the HYDRATED path (`hydratedMessagesToChatMessages`, + * objectui#8442) carried `approval`, the LIVE path here dropped it. + * + * ⚠️ `approval` and `pendingActionId` are NOT the same thing and neither + * replaces the other: the first is the runtime's own request id, the second is + * the ObjectStack `pending_actions` row the approve/reject endpoints take. The + * last test below pins that they ride together. + */ + +import { describe, it, expect } from 'vitest'; +import { uiMessageToChatMessage } from '../mapMessages'; + +describe('extractToolInvocations lifts the approval envelope', () => { + it('carries id, decision, reason and the provider flags', () => { + const out = uiMessageToChatMessage({ + id: 'm1', + role: 'assistant', + parts: [ + { + type: 'tool-drop_table', + toolCallId: 'c1', + state: 'approval-responded', + input: { table: 't' }, + approval: { + id: 'apr_1', + approved: false, + reason: 'too risky', + isAutomatic: true, + signature: 'sig', + }, + }, + ], + } as never); + + expect(out.toolInvocations?.[0]).toMatchObject({ + toolName: 'drop_table', + state: 'approval-responded', + approval: { + id: 'apr_1', + approved: false, + reason: 'too risky', + isAutomatic: true, + signature: 'sig', + }, + }); + }); + + it('LIT CONTROL — a part with no envelope yields no envelope', () => { + // Without this, a lift that hard-coded an object would satisfy the test + // above. An invented `approval.id` is precisely the fabrication the + // contract-first rule forbids. + const out = uiMessageToChatMessage({ + id: 'm2', + role: 'assistant', + parts: [ + { type: 'tool-search', toolCallId: 'c2', state: 'output-available', output: { hits: 0 } }, + ], + } as never); + + expect(out.toolInvocations?.[0]?.approval).toBeUndefined(); + }); + + it('refuses an envelope whose id cannot be replied on', () => { + // `id` is required by the output contract. An envelope without a usable one + // is not an envelope — lifting it would hand a chat surface an approval it + // can never answer. + for (const approval of [{}, { id: '' }, { id: 42 }, { approved: true }]) { + const out = uiMessageToChatMessage({ + id: 'm3', + role: 'assistant', + parts: [ + { type: 'tool-x', toolCallId: 'c3', state: 'approval-requested', input: {}, approval }, + ], + } as never); + expect(out.toolInvocations?.[0]?.approval).toBeUndefined(); + } + }); + + it('rides ALONGSIDE pendingActionId rather than replacing it', () => { + const out = uiMessageToChatMessage({ + id: 'm4', + role: 'assistant', + parts: [ + { + type: 'tool-delete_records', + toolCallId: 'c4', + state: 'approval-requested', + input: { id: 'r1' }, + output: { status: 'pending_approval', pendingActionId: 'pa_9' }, + approval: { id: 'apr_9' }, + }, + ], + } as never); + + expect(out.toolInvocations?.[0]).toMatchObject({ + state: 'approval-requested', + approval: { id: 'apr_9' }, + pendingActionId: 'pa_9', + }); + }); +}); diff --git a/packages/plugin-chatbot/src/__tests__/useObjectChat.discriminatedParts-8426.test.tsx b/packages/plugin-chatbot/src/__tests__/useObjectChat.discriminatedParts-8426.test.tsx new file mode 100644 index 0000000000..789225501d --- /dev/null +++ b/packages/plugin-chatbot/src/__tests__/useObjectChat.discriminatedParts-8426.test.tsx @@ -0,0 +1,316 @@ +/** + * 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. + */ + +/** + * `useObjectChat` — the API-mode parts builder CONSTRUCTS the discriminated + * tool part the chat runtime declares, so the `as any` on the `messages` + * option is gone (objectui#8426). + * + * ## What this file can and cannot prove + * + * The load-bearing proof of this card is a TYPE-CHECK: the builder's array is + * declared as the store's own message type, so a part that does not fit is a + * compile error at the branch that built it. ⚠️ vitest cannot see any of that — + * types are erased before a test runs, and a suite that only inspected the + * emitted objects would pass identically on the pre-fix builder, whose parts + * were plain `Record`s. + * + * So the assertions here are deliberately RUNTIME ones, chosen because they + * fail on the pre-fix output: + * + * - `validateUIMessages` is the chat runtime's OWN statement of what its + * store may hold. It is the same instrument objectui#8443 used, and it is + * what a backend route runs over these values. Each subject below is + * paired with a control built the way the OLD builder built it, so a + * subject that stops discriminating cannot go quiet — the control is what + * notices. + * - the approval envelope is DATA, not a type: the three approval states are + * unreachable without it, so "the state survived the seed" is a claim a + * test can make. + */ + +import { renderHook } from '@testing-library/react'; +import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { validateUIMessages } from 'ai'; +import { useObjectChat } from '../useObjectChat'; + +const API = 'https://example.test/api/v1/ai/agents/build/chat'; + +type SeededPart = Record & { type?: string }; +type SeededMessage = { id: string; role: string; parts: SeededPart[] }; + +const { seen } = vi.hoisted(() => ({ seen: { messages: undefined as SeededMessage[] | undefined } })); + +vi.mock('@ai-sdk/react', () => ({ + // `messages` IS what `useChat` initialises its store from, so capturing the + // option reads exactly the seed the store receives. + useChat: (options: { messages?: SeededMessage[] }) => { + seen.messages = options?.messages; + return { + messages: [], + status: 'ready', + error: undefined, + sendMessage: vi.fn(), + regenerate: vi.fn(), + stop: vi.fn(), + setMessages: vi.fn(), + }; + }, +})); + +beforeEach(() => { + seen.messages = undefined; +}); + +/** Seed one assistant message carrying exactly one tool invocation. */ +function seedOneInvocation(invocation: Record): SeededPart { + renderHook(() => + useObjectChat({ + api: API, + initialMessages: [ + { + id: 'm-tool', + role: 'assistant', + content: '', + toolInvocations: [invocation], + }, + ], + } as never), + ); + const parts = seen.messages?.[0]?.parts ?? []; + expect(parts).toHaveLength(1); + return parts[0]; +} + +describe('the three approval states are CONSTRUCTED, envelope and all', () => { + it('approval-requested keeps its state and carries the envelope id', async () => { + const part = seedOneInvocation({ + toolCallId: 'call-1', + toolName: 'delete_records', + args: { id: 'r1' }, + state: 'approval-requested', + approval: { id: 'apr_1', isAutomatic: false }, + }); + + expect(part).toMatchObject({ + type: 'tool-delete_records', + toolCallId: 'call-1', + state: 'approval-requested', + approval: { id: 'apr_1' }, + }); + // SUBJECT: the runtime's own validator accepts the seed. + await expect(validateUIMessages({ messages: seen.messages as never })).resolves.toHaveLength(1); + + // CONTROL: the SAME state with the envelope stripped — the shape the old + // builder emitted — is refused. Without this, a builder that dropped + // `approval` entirely would still satisfy the subject. + const stripped = [ + { ...seen.messages![0], parts: [{ ...part, approval: undefined }] }, + ]; + await expect(validateUIMessages({ messages: stripped as never })).rejects.toThrow(); + }); + + it('approval-responded carries the decision', async () => { + const part = seedOneInvocation({ + toolCallId: 'call-2', + toolName: 'publish_app', + args: { app: 'crm' }, + state: 'approval-responded', + approval: { id: 'apr_2', approved: true, reason: 'looks right' }, + }); + + expect(part).toMatchObject({ + type: 'tool-publish_app', + state: 'approval-responded', + approval: { id: 'apr_2', approved: true, reason: 'looks right' }, + }); + await expect(validateUIMessages({ messages: seen.messages as never })).resolves.toHaveLength(1); + }); + + it('output-denied pins `approved: false`', async () => { + const part = seedOneInvocation({ + toolCallId: 'call-3', + toolName: 'drop_table', + args: { table: 't' }, + state: 'output-denied', + approval: { id: 'apr_3', approved: false, reason: 'no' }, + }); + + expect(part).toMatchObject({ + type: 'tool-drop_table', + state: 'output-denied', + approval: { id: 'apr_3', approved: false }, + }); + await expect(validateUIMessages({ messages: seen.messages as never })).resolves.toHaveLength(1); + }); + + it('a DYNAMIC tool name reaches the store as the `tool-` discriminant', () => { + // The card feared this was the blocker. It is not: the runtime's tool set + // is open, so the discriminant is a template and any name fits. + const part = seedOneInvocation({ + toolCallId: 'call-4', + toolName: 'a_tool_named_at_runtime', + args: {}, + state: 'input-available', + }); + expect(part.type).toBe('tool-a_tool_named_at_runtime'); + }); +}); + +describe('the dead `toolName` excess property is gone', () => { + it('is not written onto a `tool-*` part, and the name is still recoverable', async () => { + const part = seedOneInvocation({ + toolCallId: 'call-5', + toolName: 'search', + args: { q: 'x' }, + result: { hits: 1 }, + state: 'output-available', + }); + + // Only the dynamic-tool arm declares `toolName`; on a `tool-*` part it was + // an excess property AND dead — the round-trip reader derives the name off + // `type`, which is why dropping it is behaviour-preserving. + expect(part).not.toHaveProperty('toolName'); + expect(part.type).toBe('tool-search'); + await expect(validateUIMessages({ messages: seen.messages as never })).resolves.toHaveLength(1); + }); +}); + +describe('the legacy authoring states are FOLDED onto the lifecycle', () => { + it.each([ + ['partial-call', 'input-streaming'], + ['call', 'input-available'], + ['result', 'output-available'], + ])('%s -> %s', async (authored, expected) => { + const part = seedOneInvocation({ + toolCallId: `call-${authored}`, + toolName: 'legacy', + args: { a: 1 }, + result: authored === 'result' ? { ok: true } : undefined, + state: authored, + }); + + expect(part.state).toBe(expected); + // SUBJECT: folded, the seed is holdable. + await expect(validateUIMessages({ messages: seen.messages as never })).resolves.toHaveLength(1); + + // CONTROL: the authored spelling passed through verbatim — what the old + // builder did — is not a state the store knows. + const passthrough = [ + { ...seen.messages![0], parts: [{ ...part, state: authored }] }, + ]; + await expect(validateUIMessages({ messages: passthrough as never })).rejects.toThrow(); + }); +}); + +describe('an approval state with nothing to back it is REPORTED, not invented', () => { + let warn: ReturnType; + + beforeEach(() => { + warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + }); + afterEach(() => { + warn.mockRestore(); + }); + + it('derives the state from the data and names the producer', async () => { + // ⛔ No `approval.id` is invented — inventing contract data is what + // AGENTS.md #0.1 forbids. The state is read off what the invocation DOES + // carry, and the author is told once. + const part = seedOneInvocation({ + toolCallId: 'call-6', + // A distinct tool name per case: the warning is deduped per (state, tool). + toolName: 'unbacked_a', + args: { a: 1 }, + state: 'approval-requested', + }); + + expect(part.state).toBe('input-available'); + expect(part).not.toHaveProperty('approval'); + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0]?.[0])).toMatch(/approval.*envelope/is); + await expect(validateUIMessages({ messages: seen.messages as never })).resolves.toHaveLength(1); + }); + + it('says it ONCE for the same state and tool', () => { + seedOneInvocation({ + toolCallId: 'call-7a', + toolName: 'unbacked_b', + args: {}, + state: 'approval-responded', + }); + seedOneInvocation({ + toolCallId: 'call-7b', + toolName: 'unbacked_b', + args: {}, + state: 'approval-responded', + }); + expect(warn).toHaveBeenCalledTimes(1); + }); + + it('⛔ says NOTHING when the invocation is an ObjectStack HITL approval', () => { + // A `pendingActionId` approval is carried by that id and by the + // `{ status: 'pending_approval' }` result, never by the runtime envelope — + // and `mapMessages` re-promotes the state from that result on the way back + // out. Warning here would cry wolf on the live HITL path. + const part = seedOneInvocation({ + toolCallId: 'call-8', + toolName: 'hitl_tool', + args: {}, + result: { status: 'pending_approval', pendingActionId: 'pa_1' }, + state: 'approval-requested', + pendingActionId: 'pa_1', + }); + + expect(warn).not.toHaveBeenCalled(); + // The result rides along, which is what lets the state be re-derived. + expect(part).toMatchObject({ + state: 'output-available', + output: { status: 'pending_approval', pendingActionId: 'pa_1' }, + }); + }); +}); + +describe('the whole seed is holdable — the end of the cast', () => { + it('a mixed message (text + reasoning + tools) passes the runtime validator', async () => { + renderHook(() => + useObjectChat({ + api: API, + initialMessages: [ + { id: 'm-user', role: 'user', content: 'do it' }, + { + id: 'm-assistant', + role: 'assistant', + content: 'working on it', + reasoning: 'thinking', + toolInvocations: [ + { toolCallId: 'c1', toolName: 'search', args: { q: 'x' }, state: 'input-available' }, + { + toolCallId: 'c2', + toolName: 'write', + args: { v: 1 }, + errorText: 'boom', + state: 'output-error', + }, + ], + }, + ], + } as never), + ); + + const seeded = seen.messages ?? []; + expect(seeded).toHaveLength(2); + expect(seeded[1]?.parts.map((p) => p.type)).toEqual([ + 'text', + 'reasoning', + 'tool-search', + 'tool-write', + ]); + await expect(validateUIMessages({ messages: seeded as never })).resolves.toHaveLength(2); + }); +}); diff --git a/packages/plugin-chatbot/src/mapMessages.ts b/packages/plugin-chatbot/src/mapMessages.ts index 4e202c7da7..19c72cc270 100644 --- a/packages/plugin-chatbot/src/mapMessages.ts +++ b/packages/plugin-chatbot/src/mapMessages.ts @@ -39,6 +39,19 @@ interface AnyPart { * output checked; this stays open. */ state?: string; + /** + * The chat runtime's approval envelope, as it arrives on a tool part. Typed + * loosely like the rest of this interface — `id` is REQUIRED by the output + * contract but optional here, so a producer that omits it is refused by + * {@link liftApproval} rather than by the shape of the input interface. + */ + approval?: { + id?: string; + approved?: boolean; + reason?: string; + isAutomatic?: boolean; + signature?: string; + }; url?: string; href?: string; title?: string; @@ -642,6 +655,39 @@ export function buildProgressFromDraftReview( }; } +/** + * Lift the chat runtime's approval envelope off a tool part. + * + * The envelope is what makes the three approval states ACTIONABLE rather than + * merely displayable: the runtime's own tool-part union makes it required + * alongside `approval-requested`, `approval-responded` and `output-denied`, so + * a producer that rebuilds a part from an invocation without it cannot + * reconstruct those states at all. This is the live half of objectui#8426 — + * `hydratedMessagesToChatMessages` already lifts it on the HYDRATED half + * (objectui#8442), and until this landed the two paths disagreed about the + * same conversation. + * + * ⚠️ NOT a replacement for `pendingActionId`, which rides alongside it: that is + * the ObjectStack `pending_actions` row the approve/reject endpoints take, + * while this is the runtime's own request id. + * + * A missing or empty `id` means there is no envelope to lift — an envelope + * whose id cannot be replied on is not one. + */ +function liftApproval(part: AnyPart): ChatToolInvocation['approval'] { + const approval = part.approval; + if (!approval) return undefined; + const { id } = approval; + if (typeof id !== 'string' || id.length === 0) return undefined; + return { + id, + approved: approval.approved, + reason: approval.reason, + isAutomatic: approval.isAutomatic, + signature: approval.signature, + }; +} + function extractToolInvocations( parts: AnyPart[], opts: { liveTail?: boolean } = {}, @@ -702,6 +748,7 @@ function extractToolInvocations( result, errorText: p.errorText, state, + approval: liftApproval(p), pendingActionId: pending?.pendingActionId, draftReview, proposedPlan, diff --git a/packages/plugin-chatbot/src/useObjectChat.ts b/packages/plugin-chatbot/src/useObjectChat.ts index 5b604f133a..d368638b46 100644 --- a/packages/plugin-chatbot/src/useObjectChat.ts +++ b/packages/plugin-chatbot/src/useObjectChat.ts @@ -13,7 +13,7 @@ import { DefaultChatTransport } from 'ai'; import { generateUniqueId } from './utils'; import { uiMessagesToChatMessages } from './mapMessages'; import { toRuntimeRole, toRuntimeTimestamp } from './chatMessageAdapter'; -import type { SeamChatMessage } from './chatMessageAdapter'; +import type { SeamChatMessage, SeamToolInvocation } from './chatMessageAdapter'; /** * What `useObjectChat` actually emits — from `messages` and from the @@ -237,7 +237,22 @@ function warnMaxToolRoundtripsInert(): void { } type InitialMessage = OuiChatMessage & { - parts?: Array>; + /** + * Pre-built chat-runtime parts, handed through to the store untouched when + * present. Declared as {@link SdkChatMessage}'s own part array — DERIVED, so + * a runtime bump moves it — rather than as the `Array>` it used to be (objectui#8426). + * + * ⚠️ **BREAKING for a host that passes `parts`.** `Record` + * admitted every object, including the ones the store cannot hold, and the + * mismatch was absorbed by a cast at the `useChat` call instead of being + * reported here. Nothing in this repository sets this member (the schema + * renderer passes `schema.messages`, and app-shell passes the output of + * `hydratedMessagesToChatMessages`, whose literal declares no `parts`), so + * the narrowing is visible only to external hosts — which is exactly the + * population it protects. + */ + parts?: SdkChatMessage['parts']; reasoning?: string; }; @@ -495,6 +510,210 @@ function narrowToSdkChatMessages(messages: unknown[]): SdkChatMessage[] { return narrowed; } +/** + * One element of the part array {@link SdkChatMessage} carries — DERIVED from + * that alias for the same reason it is itself derived: no chat-runtime type is + * named here, so a version bump that moves the part union moves this with it. + */ +type SdkMessagePart = SdkChatMessage['parts'][number]; + +/** + * The TOOL arm of that union. + * + * The runtime's tool part is a mapped type over its tool set; that set is open + * (`Record`), so the map collapses to an index signature and the + * arm's discriminant is the template `tool-${string}`. ⇒ a DYNAMIC tool name is + * fully expressible, which is the thing objectui#8426 was feared to be blocked + * on and which was measured false before this builder was written. + */ +type SdkToolPart = Extract; + +/** One warning per (state, tool) pair — see {@link warnApprovalStateWithoutEnvelope}. */ +const warnedApprovalWithoutEnvelope = new Set(); + +/** + * An invocation claims an approval state with no envelope to back it. Decide + * whether that is a producer bug worth telling the author about. + * + * ⛔ It is NOT, when the invocation carries a `pendingActionId`: an ObjectStack + * HITL approval is carried by that id and by the `{ status: 'pending_approval' }` + * tool result, never by the chat runtime's own envelope, and + * `mapMessages.extractToolInvocations` RE-PROMOTES the state from that same + * result on the way back out. So the approval card survives the round trip + * through the derived arm, and there is nothing to report. + */ +function reportUnbackedApprovalState(tool: SeamToolInvocation): void { + if (tool.pendingActionId) return; + warnApprovalStateWithoutEnvelope(tool.state ?? 'approval', tool.toolName); +} + +/** + * Tell an author once that an invocation claims an approval state it cannot + * back up. + * + * The chat runtime makes the `approval` envelope REQUIRED alongside + * `approval-requested`, `approval-responded` and `output-denied`: a value that + * claims one of those states without it is not a constructible part, so there + * is no faithful thing to build. This is NOT a tolerated dialect (AGENTS.md + * #0.1) — the producer is wrong and is told so; the state is then derived from + * the data the invocation DOES carry so the turn still renders. + * + * The authoring `state` union shedding these three runtime-only states is the + * residual clause of this chain's ruling and is deliberately not done in this + * package; once it lands, this branch becomes unreachable by construction and + * goes away with it. See `ChatToolInvocation` in `@object-ui/types`, whose own + * doc records that the narrowing was left to objectui#8426. + */ +function warnApprovalStateWithoutEnvelope(state: string, toolName: string): void { + const key = `${state}:${toolName}`; + if (warnedApprovalWithoutEnvelope.has(key)) return; + warnedApprovalWithoutEnvelope.add(key); + console.warn( + `[@object-ui/plugin-chatbot] tool invocation \`${toolName}\` declares state ` + + `\`${state}\` with no \`approval\` envelope. The chat runtime requires the ` + + 'envelope alongside that state, so the invocation is not representable as ' + + 'authored and its state was derived from the data instead. Fix it at the ' + + 'producer: carry `approval.id` (plus `approved` for `approval-responded` ' + + 'and `output-denied`) beside the state (objectui#8426).', + ); +} + +/** + * The arm to build when the declared state is absent, or is an approval state + * with no envelope to back it: read it off the data the invocation carries. + * + * `input-available` is the floor rather than "no state at all" because the + * runtime's tool part has no state-less arm — every arm carries one. It is + * also the honest reading of "input, no output yet", and `mapMessages` already + * promotes a dangling `input-*` in non-live history to a terminal state, so a + * reloaded conversation does not show it spinning. + */ +function deriveSdkToolPart(tool: SeamToolInvocation, type: `tool-${string}`): SdkToolPart { + const { toolCallId } = tool; + const input = tool.args; + if (tool.errorText !== undefined) { + return { type, toolCallId, state: 'output-error', input, errorText: tool.errorText }; + } + if (tool.result !== undefined) { + return { type, toolCallId, state: 'output-available', input, output: tool.result }; + } + return { type, toolCallId, state: 'input-available', input }; +} + +/** + * Build ONE discriminated tool part from one chat tool invocation. + * + * This is objectui#8426's clause of the chain's ruling: the producer + * CONSTRUCTS the discriminated shape, so the `as any` that used to sit on the + * `messages` option at the `useChat` call below is no longer load-bearing and + * is gone. Two consequences worth naming, because both were measured rather + * than assumed: + * + * - **`toolName` is not copied across.** It is an excess property on a + * `tool-*` part (only the dynamic-tool arm declares one) AND it is dead on + * this path: the round-trip reader in `mapMessages.ts` derives the name by + * stripping the `tool-` prefix off `type`, and reads a part's `toolName` + * only for a `dynamic-tool` part. Dropping it is behaviour-preserving. + * - **the legacy authoring states are FOLDED, not passed through.** + * `partial-call` / `call` / `result` are not runtime states; passing them + * through left `isToolState` in `mapMessages.ts` refusing them, so the + * invocation came back with no state at all. Folding them onto the + * lifecycle arm each one means is what the authoring contract's own doc + * says they map to. + */ +function toSdkToolPart(tool: SeamToolInvocation): SdkToolPart { + const type: `tool-${string}` = `tool-${tool.toolName}`; + const { toolCallId, approval } = tool; + const input = tool.args; + + switch (tool.state) { + case 'approval-requested': + // `approved` and `reason` are `?: never` on this arm — an OUTSTANDING + // request has neither, so neither is copied across even if a producer + // put one there. + if (approval) { + return { + type, + toolCallId, + state: 'approval-requested', + input, + approval: { + id: approval.id, + isAutomatic: approval.isAutomatic, + signature: approval.signature, + }, + }; + } + reportUnbackedApprovalState(tool); + break; + case 'approval-responded': + // The decision itself is what this state MEANS, so an envelope without + // one does not back it either. + if (approval && typeof approval.approved === 'boolean') { + return { + type, + toolCallId, + state: 'approval-responded', + input, + approval: { + id: approval.id, + approved: approval.approved, + reason: approval.reason, + isAutomatic: approval.isAutomatic, + signature: approval.signature, + }, + }; + } + reportUnbackedApprovalState(tool); + break; + case 'output-denied': + // This arm pins `approved: false`. An envelope saying `true` contradicts + // the state it is attached to, so it does not back it up. + if (approval && approval.approved === false) { + return { + type, + toolCallId, + state: 'output-denied', + input, + approval: { + id: approval.id, + approved: false, + reason: approval.reason, + isAutomatic: approval.isAutomatic, + signature: approval.signature, + }, + }; + } + reportUnbackedApprovalState(tool); + break; + case 'input-streaming': + case 'partial-call': + return { type, toolCallId, state: 'input-streaming', input }; + case 'input-available': + case 'call': + return { type, toolCallId, state: 'input-available', input }; + case 'output-error': + // `errorText` is required on this arm. An authored error with no text is + // still an error: the state is kept and the empty message is the + // author's own. + return { type, toolCallId, state: 'output-error', input, errorText: tool.errorText ?? '' }; + case 'output-available': + case 'result': + return { type, toolCallId, state: 'output-available', input, output: tool.result }; + case undefined: + break; + default: { + // Exhaustiveness. A state added to the authoring union lands here and + // turns this assignment red, instead of silently taking the derived arm. + const unhandledState: never = tool.state; + void unhandledState; + break; + } + } + + return deriveSdkToolPart(tool, type); +} + /** * useObjectChat – Composable hook for ObjectUI Chatbot. * @@ -565,9 +784,16 @@ export function useObjectChat(options: UseObjectChatOptions = {}): UseObjectChat // the render seam already applies. This builder now performs it instead of // asserting it, same as objectui#4424 and objectui#8342 each did for one other // instance of this class. - const aiInitialMessages = useMemo( + // + // The array this builds is DECLARED as what the store takes + // (`SdkChatMessage[]`), and every part is CONSTRUCTED to fit — the + // objectui#8426 clause of the ruling. The annotation is on the map callback + // rather than on the `useMemo` alone so a builder branch that stops fitting + // is reported at the branch that broke, not at the call site that consumes + // it. + const aiInitialMessages = useMemo( () => - (initialMessages ?? []).map((msg, idx) => { + (initialMessages ?? []).map((msg, idx): SdkChatMessage => { if (Array.isArray(msg.parts) && msg.parts.length > 0) { return { id: msg.id || `msg-${idx}`, @@ -576,7 +802,7 @@ export function useObjectChat(options: UseObjectChatOptions = {}): UseObjectChat }; } const normalized = normalizeMessages([msg])[0]; - const parts: Array> = []; + const parts: SdkChatMessage['parts'] = []; if (normalized.content) { parts.push({ type: 'text', text: normalized.content }); } @@ -584,15 +810,7 @@ export function useObjectChat(options: UseObjectChatOptions = {}): UseObjectChat parts.push({ type: 'reasoning', text: msg.reasoning }); } for (const tool of normalized.toolInvocations ?? []) { - parts.push({ - type: `tool-${tool.toolName}`, - toolCallId: tool.toolCallId, - toolName: tool.toolName, - input: tool.args, - output: tool.result, - errorText: tool.errorText, - state: tool.state, - }); + parts.push(toSdkToolPart(tool)); } return { id: normalized.id || `msg-${idx}`, @@ -724,15 +942,14 @@ export function useObjectChat(options: UseObjectChatOptions = {}): UseObjectChat const chatRef = useRef(null); const chatResult = useChat({ transport, - // The `as any` here is the LIVE suppression on this call, and it is the only - // one: `aiInitialMessages` builds `parts` as `Record[]`, which - // is not a `UIMessagePart` union, so dropping this cast turns the call red with - // TS2322 (measured, objectui#8378). The blanket `as any` that used to sit on the - // whole options object was removed there because it hid nothing this one does not - // already absorb — but it also switched off checking of `transport`, `onError` - // and excess properties. Fix the builder before deleting this cast; do NOT - // re-widen the call by casting the options object again. - messages: isApiMode && aiInitialMessages.length > 0 ? (aiInitialMessages as any) : undefined, + // No cast. `aiInitialMessages` is built as `SdkChatMessage[]` and every + // part is constructed to fit the store's own part union, so this option is + // CHECKED — which is the whole of objectui#8426 (the last suppression on + // this call; the blanket `as any` on the options object went with + // objectui#8378). ⛔ Do not re-widen this call, here or on the options + // object: a mismatch belongs at the producer above, where the branch that + // caused it is named. + messages: isApiMode && aiInitialMessages.length > 0 ? aiInitialMessages : undefined, onError: isApiMode ? (err: Error) => { // The POST was rejected before any reply streamed (see sendAwareFetch). From 601b32540c091334fcd4ee5b2ad1808b15ce585f Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 19 Sep 2026 11:37:35 +0000 Subject: [PATCH 2/2] fix(plugin-chatbot): keep the mapper's absorbing input interface uniformly permissive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `AnyPart.approval` arrived typed as the envelope's declared shape. That made the deliberately-permissive input interface structured on one member — the same mistake, one member over, that the earlier widening of `state` was filed to undo, and it falsified that pending declaration's own sentence that "every other member is `string` / `unknown`". So the member is `unknown` like its neighbours, and `liftApproval` does the narrowing — which is where the output check belongs and which the interface's doc already says. Each member is checked on its own, so a producer that gets one of them wrong loses that member rather than the whole envelope: the `id` is what makes an approval answerable, and it is established first. Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_018HrVaotisyhgmot9o2MLRq --- .../mapMessages.approvalLift-8426.test.ts | 30 ++++++++++++- packages/plugin-chatbot/src/mapMessages.ts | 44 +++++++++++-------- 2 files changed, 55 insertions(+), 19 deletions(-) diff --git a/packages/plugin-chatbot/src/__tests__/mapMessages.approvalLift-8426.test.ts b/packages/plugin-chatbot/src/__tests__/mapMessages.approvalLift-8426.test.ts index b0adef0ecf..1941b243d2 100644 --- a/packages/plugin-chatbot/src/__tests__/mapMessages.approvalLift-8426.test.ts +++ b/packages/plugin-chatbot/src/__tests__/mapMessages.approvalLift-8426.test.ts @@ -83,7 +83,7 @@ describe('extractToolInvocations lifts the approval envelope', () => { // `id` is required by the output contract. An envelope without a usable one // is not an envelope — lifting it would hand a chat surface an approval it // can never answer. - for (const approval of [{}, { id: '' }, { id: 42 }, { approved: true }]) { + for (const approval of [{}, { id: '' }, { id: 42 }, { approved: true }, 'apr_1', null]) { const out = uiMessageToChatMessage({ id: 'm3', role: 'assistant', @@ -95,6 +95,34 @@ describe('extractToolInvocations lifts the approval envelope', () => { } }); + it('drops a member a producer got wrong, and keeps the rest', () => { + // `AnyPart.approval` is `unknown` — the input interface absorbs whatever + // arrives, and this lift is the thing that keeps the OUTPUT checked. The + // `id` is what makes an approval answerable, so a bad sibling member costs + // that member and not the envelope. + const out = uiMessageToChatMessage({ + id: 'm3b', + role: 'assistant', + parts: [ + { + type: 'tool-x', + toolCallId: 'c3b', + state: 'approval-responded', + input: {}, + approval: { id: 'apr_x', approved: 'yes', reason: 7, isAutomatic: true }, + }, + ], + } as never); + + expect(out.toolInvocations?.[0]?.approval).toEqual({ + id: 'apr_x', + approved: undefined, + reason: undefined, + isAutomatic: true, + signature: undefined, + }); + }); + it('rides ALONGSIDE pendingActionId rather than replacing it', () => { const out = uiMessageToChatMessage({ id: 'm4', diff --git a/packages/plugin-chatbot/src/mapMessages.ts b/packages/plugin-chatbot/src/mapMessages.ts index 19c72cc270..ad666ba6da 100644 --- a/packages/plugin-chatbot/src/mapMessages.ts +++ b/packages/plugin-chatbot/src/mapMessages.ts @@ -40,18 +40,16 @@ interface AnyPart { */ state?: string; /** - * The chat runtime's approval envelope, as it arrives on a tool part. Typed - * loosely like the rest of this interface — `id` is REQUIRED by the output - * contract but optional here, so a producer that omits it is refused by - * {@link liftApproval} rather than by the shape of the input interface. + * The chat runtime's approval envelope, as it arrives on a tool part. + * + * `unknown`, like every other absorbing member here, and deliberately NOT + * the envelope's declared shape: this interface exists to take whatever a + * producer hands the mapper, and the one time a member was typed against the + * OUTPUT contract it made the permissive input interface stricter than the + * union it absorbs (objectui#8214, the `state` member). {@link liftApproval} + * is what keeps the OUTPUT checked; this stays open. */ - approval?: { - id?: string; - approved?: boolean; - reason?: string; - isAutomatic?: boolean; - signature?: string; - }; + approval?: unknown; url?: string; href?: string; title?: string; @@ -675,16 +673,26 @@ export function buildProgressFromDraftReview( * whose id cannot be replied on is not one. */ function liftApproval(part: AnyPart): ChatToolInvocation['approval'] { - const approval = part.approval; - if (!approval) return undefined; - const { id } = approval; + const { approval } = part; + if (typeof approval !== 'object' || approval === null) return undefined; + const envelope = approval as { + id?: unknown; + approved?: unknown; + reason?: unknown; + isAutomatic?: unknown; + signature?: unknown; + }; + const { id } = envelope; if (typeof id !== 'string' || id.length === 0) return undefined; + // Each member is checked on its own: a producer that gets one of them wrong + // should lose that member, not the whole envelope — the `id` is what makes + // the approval answerable, and it has already been established. return { id, - approved: approval.approved, - reason: approval.reason, - isAutomatic: approval.isAutomatic, - signature: approval.signature, + approved: typeof envelope.approved === 'boolean' ? envelope.approved : undefined, + reason: typeof envelope.reason === 'string' ? envelope.reason : undefined, + isAutomatic: typeof envelope.isAutomatic === 'boolean' ? envelope.isAutomatic : undefined, + signature: typeof envelope.signature === 'string' ? envelope.signature : undefined, }; }