diff --git a/.changeset/ai-build-thread-survives-preview-switch-2627.md b/.changeset/ai-build-thread-survives-preview-switch-2627.md new file mode 100644 index 0000000000..87b5402d24 --- /dev/null +++ b/.changeset/ai-build-thread-survives-preview-switch-2627.md @@ -0,0 +1,15 @@ +--- +'@object-ui/app-shell': patch +--- + +The AI build conversation no longer blanks itself the moment the preview opens + +`useChatConversation` treated every failed resolve the same way: clear the id, clear the messages. For a FIRST resolve that is right — there is nothing to lose. For a re-resolve of the conversation the hook is already holding it is destructive, and the AI build flow fires exactly such a re-resolve at the worst possible moment. + +The sequence is the magic-moment one. A build turn streams; `apply_blueprint`'s draft lands and the Live Canvas opens, switching the page from full-screen chat to the chat|preview split; the turn ends; ADR-0057 A1.b bind-on-create — which deliberately waits for that edge — re-keys the conversation to `app::build` and navigates to `?package=`. The scope flip re-resolves the same conversation, one GET issued at the instant the server is still finishing the heaviest turn of the session. A 502 or a dropped connection on that single request landed in the blanket catch. + +Clearing the id there is not a conservative fallback, because of what the host does with it: `AiChatPage` keys its chat pane on `` `${chatApi}:${conversationId ?? 'pending'}` ``, and the thread itself lives inside the chat hook's instance (`useObjectChat` seeds from `initialMessages` once per mount). So `undefined` does not re-render the pane, it REPLACES it, and the blueprint card, the build summary and the Publish button all leave with the discarded instance — the reported "the whole conversation went blank right after the build finished, and only came back after switching threads and back". + +A failed resolve now keeps whatever it was re-reading, when that is the conversation already held: the id is still valid and the messages are still the truth, so the surface stays as it was and the next resolve recovers. This is the other half of a guard that was already there for the empty case — the same re-resolve returning NO messages mid-turn was already refused the right to wipe hydrated history; only the failing case was still open. A resolve aimed at a DIFFERENT conversation (a sidebar switch) and a first resolve with nothing held still clear, and both are pinned negatively. + +Pinned at two levels: the hook, and the page driving the real build→preview→re-key sequence and asserting the pane is never remounted across it. diff --git a/packages/app-shell/src/console/ai/__tests__/AiChatPage.buildHistorySurvives.test.tsx b/packages/app-shell/src/console/ai/__tests__/AiChatPage.buildHistorySurvives.test.tsx new file mode 100644 index 0000000000..1131128f47 --- /dev/null +++ b/packages/app-shell/src/console/ai/__tests__/AiChatPage.buildHistorySurvives.test.tsx @@ -0,0 +1,296 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#2627 — the conversation history must survive the build→preview + * transition, including when the re-key refetch that transition fires fails. + * + * The magic-moment sequence this drives is the reported one: a build turn + * streams, `apply_blueprint`'s draft lands (Live Canvas opens — the full-screen + * chat becomes the chat|preview split), the turn ends, and the A1.b bind-on- + * create effect re-keys the conversation to `app::build` and puts + * `?package=` on the URL. That scope flip re-resolves the SAME conversation — + * one more GET, fired at the instant the server is still finishing the heaviest + * turn of the session. + * + * What made that refetch load-bearing is the pane key: `AiChatPage` mounts + * ``, and the + * thread itself lives inside the chat hook's instance (`useObjectChat` seeds + * from `initialMessages` ONCE — `aiInitialMessages` has `[]` deps and useChat's + * Chat object is created once per mount). So anything that makes + * `conversationId` go momentarily `undefined` does not merely re-render the + * pane, it REPLACES it, and every message goes with the old instance — + * "blueprint card, summary and Publish button all gone, only the composer + * left, until you switch threads and back". + * + * `useChatConversation` already refused to let an EMPTY re-read wipe hydrated + * messages; a FAILED one still cleared the id. This pins both the happy path + * and the failing-refetch path through the real page. + * + * The chat hook is faked, deliberately — but faked to the property that makes + * this bug possible: messages live in the hook INSTANCE and are seeded once at + * mount. A remount is therefore visible as lost history, exactly as in + * production. + */ + +import '@testing-library/jest-dom/vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { act, cleanup, render, screen, waitFor } from '@testing-library/react'; +import { MemoryRouter, Route, Routes } from 'react-router-dom'; +import React from 'react'; + +interface FakeMsg { + id: string; + role: string; + content: string; + toolInvocations?: unknown[]; +} + +/** One entry per `useObjectChat` MOUNT — a remount is what loses the thread. */ +const paneMounts: string[] = []; +/** Drives the faked chat hook from the test body. */ +const chat = { + isLoading: true, + /** Appends a live turn INSIDE the mounted instance (lost on a remount). */ + append: undefined as ((m: FakeMsg) => void) | undefined, +}; +let capturedProps: Record = {}; + +vi.mock('@object-ui/plugin-chatbot', async (importOriginal) => { + const actual = await importOriginal>(); + const React2 = await import('react'); + return { + ...actual, + useAgents: () => ({ + agents: [{ name: 'metadata_assistant', label: 'Build', capabilities: ['build'] }], + loading: false, + error: undefined, + refetch: vi.fn(), + }), + useAiModels: () => ({ models: [], defaultModelId: undefined }), + useHitlInChat: () => ({ decide: vi.fn(), decisions: {} }), + useObjectChat: (opts: { initialMessages?: FakeMsg[] }) => { + // Seeded ONCE, like the real hook: the thread belongs to this instance. + const [messages, setMessages] = React2.useState( + () => (opts.initialMessages ?? []) as FakeMsg[], + ); + React2.useEffect(() => { + paneMounts.push('mount'); + return () => { + paneMounts.push('unmount'); + }; + }, []); + chat.append = (m: FakeMsg) => setMessages((prev) => [...prev, m]); + return { + messages, + isLoading: chat.isLoading, + error: undefined, + sendMessage: vi.fn(), + stop: vi.fn(), + reload: vi.fn(), + clear: vi.fn(), + setMessages: vi.fn(), + }; + }, + ChatbotEnhanced: (props: Record) => { + capturedProps = props; + const msgs = (props.messages ?? []) as FakeMsg[]; + return ( +
+ {msgs.map((m) => m.id).join(',')} +
+ ); + }, + }; +}); + +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 }; +}); +// The rail runs its own listing fetch and is not part of this invariant. +vi.mock('../ConversationsSidebar', () => ({ + ConversationsSidebar: () =>
, +})); +// The canvas is an IFRAME onto `/apps/:seg?preview=draft`; happy-dom really +// tries to load it and the rejected request fails the run. What this test needs +// from the canvas is only that the page switched INTO the split layout — the +// split handle (rendered by AiChatPage itself, not by the canvas) is the +// unmocked half of that assertion. The pane's own behaviour is LiveCanvas.test. +vi.mock('../LiveCanvas', () => ({ + LiveCanvas: () =>
, +})); + +import { AiChatPage } from '../AiChatPage'; + +// happy-dom has no matchMedia — `useIsMobile` (mobile canvas overlay) needs it. +window.matchMedia = ((query: string) => ({ + matches: false, + media: query, + onchange: null, + addEventListener: () => {}, + removeEventListener: () => {}, + addListener: () => {}, + removeListener: () => {}, + dispatchEvent: () => false, +})) as unknown as typeof window.matchMedia; + +/** The persisted thread the page hydrates from (raw `ServerConversation`). */ +const PERSISTED_TURNS = [ + { id: 'r1', role: 'user', content: [{ type: 'text', text: 'build me a CRM' }] }, + { + id: 'r2', + role: 'assistant', + content: [ + { type: 'text', text: 'Here is the plan.' }, + { type: 'tool-call', toolCallId: 't1', toolName: 'propose_blueprint' }, + ], + }, +]; + +let serverTurns: unknown[] = []; +let conversationGets = 0; +/** 1-based index of the conversation GET that should fail (0 = none fail). */ +let failGetNumber = 0; + +function installFetch(): void { + vi.stubGlobal( + 'fetch', + vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => { + const url = String(input); + const method = init?.method ?? 'GET'; + if (/\/conversations\/conv-1$/.test(url) && method === 'GET') { + conversationGets += 1; + if (conversationGets === failGetNumber) { + return new Response('upstream busy', { status: 502 }); + } + return new Response(JSON.stringify({ id: 'conv-1', messages: serverTurns }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + } + return new Response(JSON.stringify({ success: true, data: [] }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }); + }), + ); +} + +function thread(): string { + return screen.getByTestId('thread').textContent ?? ''; +} + +function mountCount(): number { + return paneMounts.filter((m) => m === 'mount').length; +} + +/** + * Drive the reported sequence and return the pane-mount count at the moment + * the preview opened, so the caller can assert nothing remounted after it. + */ +async function driveBuildToPreview(): Promise { + render( + + + } /> + } /> + + , + ); + await waitFor(() => expect(screen.getByTestId('pane')).toBeInTheDocument()); + // Hydrated: the plan card and the turn that proposed it are on screen. + await waitFor(() => expect(thread()).toBe('r1,r2')); + + // The build turn is now streaming. The server persists a turn at COMPLETION, + // so a read taken right now returns nothing — that is the race window. + serverTurns = []; + + // `apply_blueprint`'s draft lands mid-stream: the Live Canvas opens (the + // layout switches from full-screen chat to the chat|preview split) and the + // thread starts carrying the package the build just minted. + await act(async () => { + chat.append?.({ + id: 'live-build', + role: 'assistant', + content: '', + toolInvocations: [ + { toolCallId: 't2', toolName: 'apply_blueprint', draftReview: { packageId: 'app.crm' } }, + ], + }); + (capturedProps.onDraftArtifacts as (a: unknown[], seg?: string) => void)( + [{ type: 'app', name: 'crm' }], + 'app.crm', + ); + }); + // The layout switch actually happened — this is the transition under test. + // The split handle only exists in the desktop chat|preview layout. + expect(screen.getByTestId('live-canvas')).toBeInTheDocument(); + expect(screen.getByTestId('ai-chat-split-handle')).toBeInTheDocument(); + const mountsAtPreview = mountCount(); + + // The turn ends. A1.b bind-on-create waits for exactly this edge, then + // re-keys the conversation and navigates to `?package=app.crm` — the scope + // flip that fires the re-resolve of the conversation we already hold. + await act(async () => { + chat.isLoading = false; + chat.append?.({ id: 'live-summary', role: 'assistant', content: 'Your CRM is ready.' }); + }); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 50)); + }); + return mountsAtPreview; +} + +describe('AiChatPage — the thread survives the build→preview transition (#2627)', () => { + beforeEach(() => { + paneMounts.length = 0; + conversationGets = 0; + failGetNumber = 0; + chat.isLoading = true; + chat.append = undefined; + serverTurns = PERSISTED_TURNS; + localStorage.clear(); + installFetch(); + }); + + afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); + }); + + it('keeps the whole thread when the A1.b re-key refetch succeeds', async () => { + const mountsAtPreview = await driveBuildToPreview(); + + // The re-key DID re-resolve the same conversation (this is the GET the + // failing case below breaks) — otherwise the next test proves nothing. + expect(conversationGets).toBeGreaterThan(1); + expect(mountCount()).toBe(mountsAtPreview); + expect(thread()).toBe('r1,r2,live-build,live-summary'); + }); + + it('keeps the whole thread when that refetch FAILS (the blanked-pane report)', async () => { + // Fail the SECOND conversation GET: the first is the page's hydration, the + // second is the re-key's re-resolve fired as the build turn lands. + failGetNumber = 2; + + const mountsAtPreview = await driveBuildToPreview(); + + expect(conversationGets).toBe(2); + // Before the fix this remounted the pane with an empty seed: the hook + // cleared `conversationId` on the failure, the key fell back to `pending`, + // and the thread went with the discarded instance. + expect(mountCount()).toBe(mountsAtPreview); + expect(thread()).toBe('r1,r2,live-build,live-summary'); + // The plan card / summary / publish affordances all derive from these + // messages, so an empty thread is the reported "everything disappeared". + expect(thread()).not.toBe(''); + }); +}); diff --git a/packages/app-shell/src/hooks/__tests__/useChatConversation.test.tsx b/packages/app-shell/src/hooks/__tests__/useChatConversation.test.tsx index fde708379f..5a52f436b2 100644 --- a/packages/app-shell/src/hooks/__tests__/useChatConversation.test.tsx +++ b/packages/app-shell/src/hooks/__tests__/useChatConversation.test.tsx @@ -864,6 +864,130 @@ describe('useChatConversation — same-conversation empty re-read preserves hydr }); }); +// objectui#2627 — the same guard's other half. An EMPTY re-read of the held +// conversation is covered above; a FAILED one used to run into the blanket +// `catch { setConversationId(undefined) }`, and dropping the id is what the +// host turns into a pane remount (its ChatPane key is +// `${chatApi}:${conversationId ?? 'pending'}`), discarding the live thread. +// The failure that matters is the A1.b re-key's refetch, fired the instant a +// long build turn ends — precisely when the server is least likely to answer. +describe('useChatConversation — a FAILED same-conversation re-read keeps the conversation', () => { + let fetchMock: ReturnType; + + beforeEach(() => { + localStorage.clear(); + fetchMock = vi.fn(); + vi.stubGlobal('fetch', fetchMock); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('keeps the id AND the hydrated messages when the scope-change refetch 502s', async () => { + fetchMock.mockResolvedValueOnce( + jsonResponse({ + id: 'conv-a', + messages: [{ id: 'm1', role: 'user', content: 'build it' }], + }), + ); + + const { result, rerender } = renderHook( + ({ scope }: { scope: string }) => + useChatConversation({ userId: 'u1', apiBase: API_BASE, scope, activeId: 'conv-a' }), + { initialProps: { scope: 'build' } }, + ); + await waitFor(() => expect(result.current.conversationId).toBe('conv-a')); + expect(result.current.initialMessages).toHaveLength(1); + + // The re-key flips the scope; the re-resolve of the SAME id hits a busy + // server. `fetchConversation` throws on a non-404/403 !ok response. + fetchMock.mockResolvedValueOnce(new Response('upstream busy', { status: 502 })); + act(() => result.current.rekeyScope('app:crm:build')); + rerender({ scope: 'app:crm:build' }); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(result.current.conversationId).toBe('conv-a'); + expect(result.current.initialMessages).toHaveLength(1); + }); + + it('survives a rejected (network-level) refetch of the held conversation too', async () => { + fetchMock.mockResolvedValueOnce( + jsonResponse({ id: 'conv-a', messages: [{ id: 'm1', role: 'user', content: 'build it' }] }), + ); + const { result, rerender } = renderHook( + ({ scope }: { scope: string }) => + useChatConversation({ userId: 'u1', apiBase: API_BASE, scope, activeId: 'conv-a' }), + { initialProps: { scope: 'build' } }, + ); + await waitFor(() => expect(result.current.conversationId).toBe('conv-a')); + + fetchMock.mockRejectedValueOnce(new TypeError('Failed to fetch')); + act(() => result.current.rekeyScope('app:crm:build')); + rerender({ scope: 'app:crm:build' }); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(result.current.conversationId).toBe('conv-a'); + expect(result.current.initialMessages).toHaveLength(1); + }); + + it('still clears when the FAILED resolve targeted a DIFFERENT conversation', async () => { + // The negative half: a sidebar switch whose fetch fails must not leave the + // previous thread on screen under a URL that now names another one. + fetchMock.mockResolvedValueOnce( + jsonResponse({ id: 'conv-a', messages: [{ id: 'm1', role: 'user', content: 'a' }] }), + ); + const { result, rerender } = renderHook( + ({ activeId }: { activeId: string }) => + useChatConversation({ userId: 'u1', apiBase: API_BASE, scope: 'build', activeId }), + { initialProps: { activeId: 'conv-a' } }, + ); + await waitFor(() => expect(result.current.conversationId).toBe('conv-a')); + + fetchMock.mockResolvedValueOnce(new Response('upstream busy', { status: 502 })); + rerender({ activeId: 'conv-b' }); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(result.current.conversationId).toBeUndefined(); + expect(result.current.initialMessages).toHaveLength(0); + }); + + it('still clears when a FIRST resolve (nothing held yet) fails', async () => { + fetchMock.mockResolvedValueOnce(new Response('upstream busy', { status: 502 })); + const { result } = renderHook(() => + useChatConversation({ userId: 'u1', apiBase: API_BASE, scope: 'build', activeId: 'conv-a' }), + ); + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(result.current.conversationId).toBeUndefined(); + expect(result.current.initialMessages).toHaveLength(0); + }); + + it('still clears when the held conversation is GONE (404) and the replacing create fails', async () => { + // The guard covers a transport failure re-reading a LIVE conversation. A + // 404 is the server being definitive: the id is dead and its caches have + // already been cleared, so a failing create must not resurrect it on screen. + fetchMock.mockResolvedValueOnce( + jsonResponse({ id: 'conv-a', messages: [{ id: 'm1', role: 'user', content: 'a' }] }), + ); + const { result, rerender } = renderHook( + ({ scope }: { scope: string }) => + useChatConversation({ userId: 'u1', apiBase: API_BASE, scope, activeId: 'conv-a' }), + { initialProps: { scope: 'build' } }, + ); + await waitFor(() => expect(result.current.conversationId).toBe('conv-a')); + + fetchMock + .mockResolvedValueOnce(new Response('gone', { status: 404 })) // GET conv-a + .mockResolvedValueOnce(new Response('nope', { status: 500 })); // POST create + act(() => result.current.rekeyScope('app:crm:build')); + rerender({ scope: 'app:crm:build' }); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(result.current.conversationId).toBeUndefined(); + expect(result.current.initialMessages).toHaveLength(0); + }); +}); + // Security — plaintext AI-chat cache (conversation-id pointers + message // bodies) must be wiped on logout / user switch so a shared machine doesn't // leak the prior user's threads. diff --git a/packages/app-shell/src/hooks/useChatConversation.ts b/packages/app-shell/src/hooks/useChatConversation.ts index 07ce89912f..ac5d996adf 100644 --- a/packages/app-shell/src/hooks/useChatConversation.ts +++ b/packages/app-shell/src/hooks/useChatConversation.ts @@ -682,6 +682,9 @@ export function useChatConversation( setIsLoading(true); (async () => { + // Which conversation this resolve is FOR, as far as it gets. The catch + // below needs it to tell "the one we already hold" from "some other id". + let targetId: string | undefined = activeId; try { if (activeId) { const existing = await fetchConversation(apiBase, activeId); @@ -707,11 +710,17 @@ export function useChatConversation( resolvedScopeRef.current = scope; return; } - // Requested id is gone — fall through to create a fresh one. + // Requested id is gone — fall through to create a fresh one. The + // server was DEFINITIVE (404/403), so from here on we are no longer + // re-reading a conversation worth preserving: drop the catch guard's + // claim on it, or a failing create below would keep an id we have + // just been told is dead (and whose caches we just cleared). + targetId = undefined; writeCache(key, undefined); writeConversationMessagesCache(activeId, []); } else if (!forceNew) { const cached = readCache(key); + targetId = cached; // A1.b migration read: nothing cached under THIS scope yet — the // thread may predate the scope (e.g. a build conversation keyed // product-only before bind-on-create shipped). Offer the legacy @@ -774,7 +783,11 @@ export function useChatConversation( // 'fresh' + a used conversation: fall through to create a fresh // one; the used thread stays in history (writeCache below repoints // the cache to the new conversation). + targetId = undefined; } else { + // Definitive miss — same as the activeId 404 above: stop claiming + // this id, so a failing create cannot preserve a dead one. + targetId = undefined; writeCache(key, undefined); writeConversationMessagesCache(cached, []); } @@ -788,10 +801,31 @@ export function useChatConversation( resolvedForUserRef.current = userId; resolvedScopeRef.current = scope; } catch { - if (!cancelled) { - setConversationId(undefined); - setInitialMessages([]); - } + if (cancelled) return; + // objectui#2627 — the OTHER half of the same-conversation guard above. + // That one stops an EMPTY re-read from wiping hydrated messages; this + // one stops a FAILED re-read from dropping the conversation itself. + // + // The A1.b re-key fires a refetch of the id we already hold at the + // worst possible moment: the instant a long build turn ends (the bind + // effect waits for `isLoading` to fall, then re-keys and navigates + // `?package=`), when the server is still busy finishing that turn. A + // 5xx/network blip on that one GET landed here, and clearing the id is + // destructive rather than conservative: the host keys its chat pane on + // `conversationId ?? 'pending'` (AiChatPage), so `undefined` REMOUNTS + // the pane, and the live thread — which lives in the chat hook's own + // instance, not in this state — is discarded. That is the reported + // "the whole conversation went blank right after the build finished, + // and only came back after switching threads and back". + // + // The id is still valid and the messages we hold are still the truth, + // so keep both: the surface stays exactly as it was and the next + // resolve recovers. Only a resolve aimed at a DIFFERENT conversation + // (a sidebar switch, a forced-new intent) still clears — there the old + // thread genuinely no longer matches what the URL asks for. + if (targetId && targetId === conversationId) return; + setConversationId(undefined); + setInitialMessages([]); } finally { if (!cancelled) setIsLoading(false); } diff --git a/packages/plugin-chatbot/src/__tests__/ChatbotEnhanced.sendError.test.tsx b/packages/plugin-chatbot/src/__tests__/ChatbotEnhanced.sendError.test.tsx index 01daef23f8..934def000e 100644 --- a/packages/plugin-chatbot/src/__tests__/ChatbotEnhanced.sendError.test.tsx +++ b/packages/plugin-chatbot/src/__tests__/ChatbotEnhanced.sendError.test.tsx @@ -12,6 +12,7 @@ import '@testing-library/jest-dom/vitest'; import { describe, it, expect, vi, beforeEach } from 'vitest'; import { render, screen, fireEvent, waitFor } from '@testing-library/react'; import { ChatbotEnhanced } from '../ChatbotEnhanced'; +import type { ChatMessage } from '../ChatbotEnhanced'; const LABELS = { sendFailedRateLimited: 'RATE_LIMIT_MSG', @@ -118,3 +119,137 @@ describe('ChatbotEnhanced send-failure UX', () => { expect(screen.queryByTestId('chat-send-error')).not.toBeInTheDocument(); }); }); + +/** + * objectui#2627 — the ROLLBACK half of the plan card's optimistic approve. + * Clicking "Build it" flips the card to a "Building…" badge before the server + * has said anything (#2632, pinned in ChatbotEnhanced.test.tsx). When that + * approval never left the client — a 429 or an offline send, tagged `notSent` — + * the optimistic badge is a lie AND a dead end: the badge replaces the buttons, + * so with no rollback the user cannot re-approve at all and the build silently + * never starts. The happy flip was pinned on its own; this is the other side. + */ +const planMessage: ChatMessage[] = [ + { + id: 'a1', + role: 'assistant', + content: '', + toolInvocations: [ + { + toolCallId: 't1', + toolName: 'propose_blueprint', + state: 'output-available', + proposedPlan: { + summary: 'A tiny CRM', + objects: [{ name: 'contact', label: 'Contact', fieldCount: 4 }], + counts: { objects: 1, views: 1, dashboards: 0, seedData: 0 }, + questions: [], + assumptions: [], + }, + }, + ], + } as unknown as ChatMessage, +]; + +describe('ChatbotEnhanced plan approval — optimistic flip rollback (#2627)', () => { + it('rolls the "Building…" badge back to the buttons when the approval never left the client', async () => { + const onSendMessage = vi.fn(); + const { rerender } = render( + , + ); + + fireEvent.click(screen.getByTestId('proposed-plan-approve')); + expect(screen.getByTestId('proposed-plan-building')).toHaveTextContent('BUILDING_NOW'); + expect(screen.queryByTestId('proposed-plan-approve')).not.toBeInTheDocument(); + + // The approval was rejected before reaching the model (rate limit). + rerender( + , + ); + + // The card must be actionable again — nothing is building. + await waitFor(() => + expect(screen.getByTestId('proposed-plan-approve')).toBeInTheDocument(), + ); + expect(screen.queryByTestId('proposed-plan-building')).not.toBeInTheDocument(); + // …and the failure is surfaced, not swallowed. + expect(screen.getByTestId('chat-send-error')).toHaveTextContent('RATE_LIMIT_MSG'); + }); + + it('a STREAMED-response error leaves the badge alone — that approval did reach the server', async () => { + const onSendMessage = vi.fn(); + const { rerender } = render( + , + ); + fireEvent.click(screen.getByTestId('proposed-plan-approve')); + + // Not `notSent`: the turn started and the stream dropped. Re-offering + // "Build it" here invites a SECOND build of the same plan. + rerender( + , + ); + + expect(screen.getByTestId('proposed-plan-building')).toBeInTheDocument(); + expect(screen.queryByTestId('proposed-plan-approve')).not.toBeInTheDocument(); + }); + + it('a LATER send supersedes the approve — its failure must not re-open the built card', async () => { + const onSendMessage = vi.fn(); + const { rerender } = render( + , + ); + fireEvent.click(screen.getByTestId('proposed-plan-approve')); + // The approval went through; the user then types something that does not. + await submit('also add invoices', onSendMessage); + + rerender( + , + ); + + // The composer text is restored, but the plan card stays "Building…" — + // rolling it back would offer a second build of a plan already building. + await waitFor(() => expect(screen.getByTestId('chat-send-error')).toBeInTheDocument()); + expect(screen.getByTestId('proposed-plan-building')).toBeInTheDocument(); + expect(screen.queryByTestId('proposed-plan-approve')).not.toBeInTheDocument(); + }); +});