diff --git a/.changeset/aichatpage-runtime-message-seam-4437.md b/.changeset/aichatpage-runtime-message-seam-4437.md new file mode 100644 index 000000000..d53ee63a1 --- /dev/null +++ b/.changeset/aichatpage-runtime-message-seam-4437.md @@ -0,0 +1,13 @@ +--- +'@object-ui/app-shell': patch +--- + +`AiChatPage` narrows the chat hook's messages through the exported `toRuntimeMessages` adapter instead of five casts + +`useObjectChat` returns `ObjectChatMessage[]` — the shape both of its modes really produce (objectui#4424) — which is wide where local mode is wide: it keeps the authored `'tool'` role and the legacy `'partial-call'`/`'call'`/`'result'` tool states. This page wants the runtime shape, and said so five times with `messages as ChatMessage[]` (plus one `as unknown as` double cast). The narrowing was real and the casts were legal; what was missing is that a cast erases the whole difference rather than the intentional part of it, so nothing recorded which narrowing was meant and nothing could go red when it changed. + +There is now one conversion where the hook's values enter the page's runtime-typed world, memoized on `messages` exactly as the plugin's own three renderers do (objectui#4399). All five sites read it; no cast replaces them, and the `as unknown as` double cast at the bound-package derivation turned out to be unnecessary once the value is honestly typed. + +One behaviour changes, and it is the one the fold was measured for. `sanitizeChatMessagesForCache` declares its parameter's role as `'user' | 'assistant' | 'system'` — it has always asked for folded roles, and the cast is what let an unfolded `'tool'` past that declaration. Measured on a thread carrying one: the entry was cached as `role: 'tool'`, which the cache's own reader then rejects, so the message vanished on a cache-fallback reload; and because sanitize gates tool serialization on `role === 'assistant'`, that turn's tool invocations — including the re-serialized draft envelope behind "Review N changes / Publish" — never reached the cache at all. Folded, both survive the reload they exist to survive. The other two compute sites were measured to be fold-insensitive and are unchanged: `isConversationZh` reads `role === 'user'` and the text, which the fold can neither create nor destroy, and `deriveBoundPackageId` walks every message irrespective of role and reads keys the adapter spreads through. + +Nothing on screen moves today: neither a `'tool'` role nor a legacy tool state is producible on this page's own paths, since hydration yields runtime roles and API mode's values come from `mapMessages` with v6 states already. This closes the hole ahead of a value that can reach it, and makes a future vocabulary move surface as a type error at the host rather than as rendering behaviour. diff --git a/packages/app-shell/src/console/ai/AiChatPage.tsx b/packages/app-shell/src/console/ai/AiChatPage.tsx index e11a9d57a..fffe2b5b0 100644 --- a/packages/app-shell/src/console/ai/AiChatPage.tsx +++ b/packages/app-shell/src/console/ai/AiChatPage.tsx @@ -73,6 +73,10 @@ import { detectBuilderHandoff, detectProposedChanges, buildProgressFromDraftReview, + // The authoring/honest -> runtime message seam (objectui#4399 / PR #4416), + // consumed here one hop up from the plugin's own renderers (objectui#4437). + // See `runtimeMessages` below for why this host calls it exactly once. + toRuntimeMessages, type AgentDescriptor, type ChatbotEnhancedToolInvocation, // The ENHANCED message shape — the one `` renders and the @@ -1585,12 +1589,52 @@ export function ChatPane({ setMessagesRef.current = setMessages; }, [setMessages]); + // ── The one conversion: where the hook's values enter this page's + // runtime-typed world (objectui#4437) ────────────────────────────────────── + // + // `useObjectChat` returns `ObjectChatMessage[]` — the shape BOTH of its modes + // really produce (objectui#4424 / PR #4436): wide where local mode is wide, + // so it keeps the authored `'tool'` role and the legacy + // `'partial-call'`/`'call'`/`'result'` tool states. Everything below this + // line wants the RUNTIME shape instead, and used to say so with five + // `as ChatMessage[]` casts. A cast erases the whole difference rather than + // the intentional part of it: a new authored role, or a newly required + // runtime key, kept compiling at all five sites and surfaced as behaviour. + // + // `toRuntimeMessages` is the same seam the plugin's own three renderers use + // (`renderer.tsx`, objectui#4399), with each narrowing decision named and + // tested in `chatMessageAdapter.ts`: `'tool'` renders as an assistant bubble, + // legacy tool states map to their v6 equivalents, and every render-only key + // (`buildProgress`, `draftReview`, `pendingActionId`, …) is spread through. + // Memoized on `messages` exactly as those renderers do, so this array's + // identity is neither more nor less stable than the hook's own output. + // + // All four consumers below take the converted array. The one that is + // fold-SENSITIVE is the cache write, and it is sensitive in the direction it + // wants — see the comment on that effect. + const runtimeMessages = useMemo(() => toRuntimeMessages(messages), [messages]); + + // The cache write is the one measured behaviour change of objectui#4437, and + // it is a fix: `sanitizeChatMessagesForCache` declares its parameter's role as + // `'user' | 'assistant' | 'system'` — it has always ASKED for folded roles, + // and the cast is what let an unfolded `'tool'` past that declaration. Two + // consequences, both measured on a message set carrying a `'tool'` role: + // - the entry was cached as `role: 'tool'`, which `readMessageCache`'s own + // validator then rejects, so the message vanished on a cache-fallback + // reload. Folded, it restores as the assistant bubble it renders as. + // - sanitize gates tool serialization on `role === 'assistant'`, so that + // message's tool invocations — including the re-serialized draft envelope + // behind "Review N changes / Publish" — were dropped from the cache + // entirely. Folded, they survive the reload they exist to survive. + // Not reachable from this page's own paths today (hydration and `mapMessages` + // both produce runtime roles and v6 states already), so nothing on screen + // moves; this closes the hole ahead of a value that can reach it. useEffect(() => { writeConversationMessagesCache( conversationId, - sanitizeChatMessagesForCache(messages as ChatMessage[]), + sanitizeChatMessagesForCache(runtimeMessages), ); - }, [conversationId, messages]); + }, [conversationId, runtimeMessages]); // #772 — the confirm-card SEND messages must match the CONVERSATION's // language, not the console UI locale: a Chinese thread under an English UI @@ -1611,9 +1655,12 @@ export function ChatPane({ // ungated `planAnswerMessage` sent the UI locale in both directions. // `outboundAgentText` resolves all four from the `zh`/`en` packs by // conversation language and never consults the UI pack. + // Fold-insensitive, measured: the probe reads `role === 'user'` and the text, + // and the fold only maps `'tool'` -> `'assistant'` — it can neither create nor + // destroy a user turn, and it leaves `content`/`parts` untouched. const convZh = useMemo( - () => isConversationZh(messages as ChatMessage[]) || isConversationZh(initialMessages), - [messages, initialMessages], + () => isConversationZh(runtimeMessages) || isConversationZh(initialMessages), + [runtimeMessages, initialMessages], ); const outboundText = useCallback( (key: OutboundAgentTextKey, vars?: Readonly>) => @@ -1657,7 +1704,7 @@ export function ChatPane({ }, [isLoading, canvasApp]); const hitl = useHitlInChat({ - messages: messages as ChatMessage[], + messages: runtimeMessages, apiBase, continueConversation: (prompt) => { sendMessage(prompt); @@ -1753,9 +1800,11 @@ export function ChatPane({ // derivations below read fields without per-access casts. const metadataApps = apps as MetadataAppItem[] | undefined; const { appLabel } = useObjectLabel(); + // Fold-insensitive, measured: this walks every message irrespective of role + // and reads `draftReview`/`builderHandoff`, which the adapter spreads through. const boundPackageId = useMemo( - () => deriveBoundPackageId(messages as unknown as readonly PackageBearingMessage[], editPackageId), - [messages, editPackageId], + () => deriveBoundPackageId(runtimeMessages, editPackageId), + [runtimeMessages, editPackageId], ); const boundPackageLabel = useMemo(() => { if (!boundPackageId) return undefined; @@ -2032,7 +2081,7 @@ export function ChatPane({ surface="plain" maxHeight="100%" headerSlot={headerSlot} - messages={messages as ChatMessage[]} + messages={runtimeMessages} placeholder={ activeAgent ? agentRouteName(activeAgent) === 'ask' diff --git a/packages/app-shell/src/console/ai/__tests__/AiChatPage.runtimeMessageSeam.test.tsx b/packages/app-shell/src/console/ai/__tests__/AiChatPage.runtimeMessageSeam.test.tsx new file mode 100644 index 000000000..e78790004 --- /dev/null +++ b/packages/app-shell/src/console/ai/__tests__/AiChatPage.runtimeMessageSeam.test.tsx @@ -0,0 +1,342 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#4437 — `AiChatPage` narrows the chat hook's messages through the + * exported `toRuntimeMessages` adapter, not through five casts. + * + * `useObjectChat` returns `ObjectChatMessage[]` (objectui#4424 / PR #4436): the + * shape BOTH of its modes really produce, which is wide where local mode is + * wide — it keeps the authored `'tool'` role and the legacy + * `'partial-call'`/`'call'`/`'result'` tool states. This page wants the runtime + * shape, and used to say so with `messages as ChatMessage[]` at five sites. A + * cast erases the whole difference rather than the intentional part of it, so + * nothing recorded WHICH narrowing was meant and nothing could go red when it + * changed. + * + * Why these pins are worth their weight even though the page cannot produce a + * `'tool'` role today (hydration yields `HydratedUIMessage['role']`, and API + * mode's values come from `mapMessages` with v6 states): the difference between + * "the cast happens to erase this" and "the adapter decides this" is invisible + * until something reaches the page carrying one. These drive the REAL page and + * assert the adapter's named decisions at the two boundaries that consume the + * result — what `` renders, and what the reload cache keeps. + * + * The fold-sensitivity measurement itself is pinned in the second describe: the + * two compute sites that are fold-INSENSITIVE must stay that way, or the + * decision to route them through the one conversion stops being justified. + */ + +import '@testing-library/jest-dom/vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { cleanup, render, screen, waitFor } from '@testing-library/react'; +import { MemoryRouter, Route, Routes } from 'react-router-dom'; +import { readFileSync } from 'node:fs'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import React from 'react'; + +import { toRuntimeMessages, type ObjectChatMessage } from '@object-ui/plugin-chatbot'; +import { isConversationZh } from '../conversationLanguage'; + +/** What the faked hook hands the page — the honest, UNFOLDED shape. */ +let hookMessages: ObjectChatMessage[] = []; +/** The `messages` prop the page passes down to ``. */ +let renderedMessages: Array> = []; +/** The `messages` option the page passes to `useHitlInChat`. */ +let hitlMessages: Array> = []; + +vi.mock('@object-ui/plugin-chatbot', async (importOriginal) => { + const actual = await importOriginal>(); + return { + ...actual, + useAgents: () => ({ + agents: [{ name: 'metadata_assistant', label: 'Build', capabilities: ['build'] }], + loading: false, + error: undefined, + refetch: vi.fn(), + }), + useAiModels: () => ({ models: [], defaultModelId: undefined }), + useHitlInChat: (opts: { messages: Array> }) => { + hitlMessages = opts.messages; + return { decide: vi.fn(), decisions: {} }; + }, + // The honest hook: it hands out exactly what the real one declares — + // including a role and tool states the runtime type does not have. + useObjectChat: () => ({ + messages: hookMessages, + isLoading: false, + error: undefined, + sendMessage: vi.fn(), + stop: vi.fn(), + reload: vi.fn(), + clear: vi.fn(), + setMessages: vi.fn(), + }), + ChatbotEnhanced: (props: Record) => { + renderedMessages = (props.messages ?? []) as Array>; + return
; + }, + }; +}); + +vi.mock('@object-ui/auth', async (importOriginal) => { + const actual = await importOriginal>(); + return { ...actual, useAuth: () => ({ user: { id: 'u1' } }) }; +}); +vi.mock('../../../providers/MetadataProvider', async (importOriginal) => { + const actual = await importOriginal>(); + return { ...actual, useMetadata: () => ({ apps: [] }) }; +}); +vi.mock('../../../providers/AdapterProvider', async (importOriginal) => { + const actual = await importOriginal>(); + return { ...actual, useAdapter: () => null }; +}); +vi.mock('../ConversationsSidebar', () => ({ + ConversationsSidebar: () =>
, +})); +vi.mock('../LiveCanvas', () => ({ LiveCanvas: () =>
})); + +import { AiChatPage, deriveBoundPackageId } from '../AiChatPage'; + +window.matchMedia = ((query: string) => ({ + matches: false, + media: query, + onchange: null, + addEventListener: () => {}, + removeEventListener: () => {}, + addListener: () => {}, + removeListener: () => {}, + dispatchEvent: () => false, +})) as unknown as typeof window.matchMedia; + +const CACHE_KEY = 'objectstack:ai-chat-messages:conv-seam'; + +/** + * A thread carrying BOTH things the honest type keeps and the runtime type + * folds: a `'tool'`-role message, and a legacy `'result'` tool state on it. + */ +const TOOL_ROLE_THREAD: ObjectChatMessage[] = [ + { id: 'u1', role: 'user', content: 'build me a CRM' }, + { + id: 'tool-turn', + role: 'tool', + content: 'apply_blueprint finished', + toolInvocations: [ + { + toolCallId: 't1', + toolName: 'apply_blueprint', + state: 'result', + draftReview: { items: [{ type: 'object', name: 'lead' }], packageId: 'app.crm' }, + pendingActionId: 'pa-1', + }, + ], + }, +]; + +function installFetch(): void { + vi.stubGlobal( + 'fetch', + vi.fn(async (input: RequestInfo | URL) => { + const url = String(input); + // `useChatConversation` has to RESOLVE the id before the page caches + // anything under it — `writeConversationMessagesCache` is a no-op without + // a conversation. + if (/\/conversations\/conv-seam$/.test(url)) { + return new Response(JSON.stringify({ id: 'conv-seam', messages: [] }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + } + return new Response(JSON.stringify({ success: true, data: [] }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + }), + ); +} + +async function renderPage(): Promise { + render( + + + } /> + } /> + + , + ); + await waitFor(() => expect(screen.getByTestId('pane')).toBeInTheDocument()); +} + +describe("AiChatPage routes the hook's messages through toRuntimeMessages (#4437)", () => { + beforeEach(() => { + hookMessages = TOOL_ROLE_THREAD; + renderedMessages = []; + hitlMessages = []; + localStorage.clear(); + installFetch(); + }); + + afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + }); + + it("renders a 'tool' message as an assistant bubble, with its legacy tool state folded", async () => { + await renderPage(); + + // The adapter's named decision (`toRuntimeRole`): an authored `'tool'` + // message IS an assistant message by the time it reaches the components. + // Under the old cast the role arrived at `` as `'tool'` + // and only `formatMessageProps`' not-'user' fallthrough made it LOOK right. + expect(renderedMessages.map((m) => m.role)).toEqual(['user', 'assistant']); + + // `toRuntimeToolState`: the legacy spelling never reaches the components. + const tools = (renderedMessages[1]?.toolInvocations ?? []) as Array>; + expect(tools[0]?.state).toBe('output-available'); + + // The pass-through is what keeps the render-only keys alive across the + // conversion — the affordances would silently vanish if it rebuilt. + expect(tools[0]?.draftReview).toEqual({ + items: [{ type: 'object', name: 'lead' }], + packageId: 'app.crm', + }); + expect(tools[0]?.pendingActionId).toBe('pa-1'); + }); + + it('hands useHitlInChat the same converted array (it reads a runtime-only key)', async () => { + await renderPage(); + + expect(hitlMessages.map((m) => m.role)).toEqual(['user', 'assistant']); + const tools = (hitlMessages[1]?.toolInvocations ?? []) as Array>; + expect(tools[0]?.pendingActionId).toBe('pa-1'); + }); + + it('caches the tool turn as an assistant message, tool part included (the measured change)', async () => { + await renderPage(); + + await waitFor(() => expect(localStorage.getItem(CACHE_KEY)).not.toBeNull()); + const cached = JSON.parse(localStorage.getItem(CACHE_KEY) as string) as Array<{ + id: string; + role: string; + parts: Array>; + }>; + + const toolTurn = cached.find((m) => m.id === 'tool-turn'); + expect(toolTurn).toBeDefined(); + + // Cached as `'tool'` (what the cast let through), this entry is written and + // then DROPPED by `readMessageCache`'s own role validator on the way back + // in — the message disappears on a cache-fallback reload. + expect(toolTurn?.role).toBe('assistant'); + + // `sanitizeChatMessagesForCache` gates tool serialization on + // `role === 'assistant'`, so pre-fold this turn cached its text and nothing + // else: the re-serialized draft envelope behind "Review N changes / + // Publish" never made it into the cache at all. + const toolPart = toolTurn?.parts.find((p) => p.type === 'tool-apply_blueprint'); + expect(toolPart).toBeDefined(); + expect(toolPart?.state).toBe('output-available'); + expect(toolPart?.output).toMatchObject({ + status: 'drafted', + packageId: 'app.crm', + drafted: [{ type: 'object', name: 'lead' }], + }); + }); +}); + +/** + * The measurement that decided the placement, kept executable. + * + * Both sites below are reached through the ONE conversion in the page, which is + * only defensible while folding cannot change what they compute. If a future + * adapter decision makes either of them fold-sensitive, this goes red and the + * placement has to be re-argued rather than silently drifting. + */ +describe('#4437 fold-sensitivity: the two insensitive compute sites stay insensitive', () => { + const zhLastUserTurn: ObjectChatMessage[] = [ + ...TOOL_ROLE_THREAD, + { id: 'u2', role: 'user', content: '继续' }, + ]; + // The sharp case: Chinese present ONLY on the message whose role the fold + // rewrites. A folded `'tool'` becomes `'assistant'`, never `'user'`, so it + // can never start being read as the conversation's language. + const zhOnlyOnToolTurn: ObjectChatMessage[] = [ + { id: 'u1', role: 'user', content: 'go ahead' }, + { id: 'tool-turn', role: 'tool', content: '这是中文的工具输出' }, + ]; + + it('isConversationZh: same verdict before and after the fold', () => { + expect(isConversationZh(zhLastUserTurn)).toBe(true); + expect(isConversationZh(toRuntimeMessages(zhLastUserTurn))).toBe(true); + + expect(isConversationZh(TOOL_ROLE_THREAD)).toBe(false); + expect(isConversationZh(toRuntimeMessages(TOOL_ROLE_THREAD))).toBe(false); + + expect(isConversationZh(zhOnlyOnToolTurn)).toBe(false); + expect(isConversationZh(toRuntimeMessages(zhOnlyOnToolTurn))).toBe(false); + }); + + it('deriveBoundPackageId: same package before and after the fold', () => { + // Newest-wins, and the packageId here rides the message whose role the fold + // rewrites — so a role-dependence would show up as it appearing or + // disappearing from the answer. + expect(deriveBoundPackageId(TOOL_ROLE_THREAD, undefined)).toBe('app.crm'); + expect(deriveBoundPackageId(toRuntimeMessages(TOOL_ROLE_THREAD), undefined)).toBe('app.crm'); + }); +}); + +/** + * The source net — the half of this card the behavioural pins above CANNOT + * cover, and the reason it is here rather than assumed. + * + * Of the five cast sites, only two are observable: the `` prop + * and the cache write both change what they produce when the fold is skipped, + * so restoring their cast reds a test above. The other three cannot red by + * construction — `isConversationZh` and `deriveBoundPackageId` are + * fold-INSENSITIVE (that is the measurement, pinned just above), and + * `useHitlInChat` reads a key the pass-through preserves either way. A cast + * coming back at any of those three is invisible to every runtime assertion in + * this file AND to `tsc`, because a cast is precisely the instruction to stop + * checking. This net is what makes them non-silent. + * + * Follows PR #4436's precedent, including the refinement it needed: comment + * lines are stripped first, because the page NAMES the deleted expression in + * the comment explaining why it is gone (as does this test). Prose about a cast + * is not a cast. + */ +describe('#4437 source net: the five casts do not come back', () => { + const PAGE = resolve(dirname(fileURLToPath(import.meta.url)), '..', 'AiChatPage.tsx'); + const source = readFileSync(PAGE, 'utf8'); + const codeOnly = source + .split('\n') + .filter((line) => !/^\s*(\/\/|\/\*|\*)/.test(line)) + .join('\n'); + + it('finds the page it is guarding', () => { + // If the file moves or is renamed, fail here rather than pass vacuously. + expect(source).toContain('export function AiChatPage('); + }); + + it("casts the chat hook's `messages` nowhere", () => { + const casts = codeOnly.match(/\bmessages\s+as\s+\w/g) ?? []; + expect( + casts, + "packages/app-shell/src/console/ai/AiChatPage.tsx narrows the chat hook's " + + '`messages` with a cast again. The honest -> runtime seam is ' + + '`toRuntimeMessages` (objectui#4399 / PR #4416); a cast here erases every ' + + 'narrowing decision it records, at a site where two of the five consumers ' + + 'cannot show the difference at runtime (objectui#4437).', + ).toEqual([]); + }); + + it('converts exactly once, memoized on the hook output', () => { + // "One conversion where the values enter the page's runtime-typed world" is + // the card's shape, not a stylistic preference: a second conversion is a + // second array identity for the same thread, and these values feed effects + // and memos keyed on it. + expect(codeOnly.match(/toRuntimeMessages\(/g) ?? []).toHaveLength(1); + expect(codeOnly).toContain( + 'const runtimeMessages = useMemo(() => toRuntimeMessages(messages), [messages]);', + ); + }); +});