From 3c135058ce690eafea835c09ce561e3915067187 Mon Sep 17 00:00:00 2001 From: HaningZS Date: Fri, 9 Oct 2026 15:01:31 +0800 Subject: [PATCH] fix(client): restore page chat scope after StrictMode effect replay Why: - StrictMode cleanup clears the page-chat request scope without setup restoring it. - A first empty submit then leaves the dock busy without reaching save or API work. What: - Restore the current request scope in effect setup and pair cleanup with that scope. - Add component regressions for initial submit, retry, scope changes, and unmount. Risk: - Keep existing request-generation and render-time invalidation behavior unchanged. - API and Chat are mocked in UI fixtures; live provider behavior was not tested. Tests: - Regression failed on unmodified source; focused suites now pass 13 tests. - npm test: 312 tests passed across 49 files on Node 24.16.0. - npm run check-format; npm run lint; npm run typecheck; npm run build. - Chrome fixture: 1280px click, 390px Tab/Enter, and API-failure retry. Live Docs: - No setup, dependency, or public API change; no documentation update required. --- src/client/PageConversation.tsx | 3 +- tests/page-conversation.test.tsx | 210 +++++++++++++++++++++++++++++++ 2 files changed, 212 insertions(+), 1 deletion(-) create mode 100644 tests/page-conversation.test.tsx diff --git a/src/client/PageConversation.tsx b/src/client/PageConversation.tsx index 6b34c1d1..75870b1b 100644 --- a/src/client/PageConversation.tsx +++ b/src/client/PageConversation.tsx @@ -47,8 +47,9 @@ export function PageConversation({ }, [scope, onOpenChange]); useEffect(() => { const current = requests.current; + current.select(scope); return () => current.select(''); - }, []); + }, [scope]); const open = async () => { if (!dot || busy) return; setBusy(true); diff --git a/tests/page-conversation.test.tsx b/tests/page-conversation.test.tsx new file mode 100644 index 00000000..99d72541 --- /dev/null +++ b/tests/page-conversation.test.tsx @@ -0,0 +1,210 @@ +import { createElement, StrictMode, type ComponentProps } from 'react'; +import { act, create, type ReactTestRenderer } from 'react-test-renderer'; +import { afterEach, beforeEach, expect, it, vi } from 'vitest'; +import type { Conversation } from '../src/shared/types'; +const api = vi.hoisted(() => vi.fn()); +vi.mock('../src/client/api', () => ({ api })); +vi.mock('../src/client/Chat', () => ({ Chat: () => null })); +import { PageConversation } from '../src/client/PageConversation'; +import { Chat } from '../src/client/Chat'; + +const dot = { + id: 'dot-a', + name: 'Scout', + spaceId: 'space', + spaceIds: ['space'], + instructions: '', + researchAllowed: true, + memoryAllowed: true, + createdAt: 0, +}; +const thread: Conversation = { + id: 'thread-a', + dotId: dot.id, + ownerId: 'owner', + title: 'Page conversation', + createdAt: 0, +}; +const props = () => ({ + page: { + id: 'page-a', + spaceId: 'space', + parentId: null, + title: 'Page A', + content: '', + revision: 1, + createdAt: 0, + updatedAt: 0, + sourceThreadId: null, + }, + workspace: { + spaces: [], + dots: [dot], + conversations: [], + calls: [], + setup: { + intelligence: true, + model: true, + browser: false, + voice: false, + slack: 'not_configured', + missing: [], + }, + }, + paused: false, + beforeChat: vi.fn(async () => true), + onRefresh: vi.fn(), + onSchedule: vi.fn(), + onSettings: vi.fn(), + onCreateDot: vi.fn(), + onOpenChange: vi.fn(), +}); +const roots: ReactTestRenderer[] = []; +const view = (p: ComponentProps, strict = true) => + strict + ? createElement(StrictMode, null, createElement(PageConversation, p)) + : createElement(PageConversation, p); +async function mount(p: ReturnType, strict = true) { + let root!: ReactTestRenderer; + await act(async () => { + root = create(view(p, strict)); + }); + roots.push(root); + p.onOpenChange.mockClear(); + return root; +} +async function submit(root: ReactTestRenderer, prompt?: string) { + if (prompt !== undefined) + await act(async () => { + root.root.findByType('input').props.onChange({ + target: { value: prompt }, + }); + }); + await act(async () => { + root.root.findByType('form').props.onSubmit({ preventDefault() {} }); + }); +} +const sendButton = (root: ReactTestRenderer) => + root.root.findByProps({ 'aria-label': 'Send to page assistant' }); +function deferred() { + let resolve!: (value: Conversation) => void; + let reject!: (error: Error) => void; + const promise = new Promise((yes, no) => { + resolve = yes; + reject = no; + }); + return { promise, resolve, reject }; +} + +beforeEach(() => { + api.mockReset().mockResolvedValue(thread); + vi.stubGlobal('IS_REACT_ACT_ENVIRONMENT', true); +}); +afterEach(async () => { + for (const root of roots.splice(0)) await act(async () => root.unmount()); + vi.unstubAllGlobals(); +}); + +it.each([false, true])( + 'opens page chat on the first empty submit (StrictMode=%s)', + async (strict) => { + const p = props(); + const root = await mount(p, strict); + // Do not type first: an input rerender masks the effect-replay bug. + await submit(root); + expect(p.beforeChat).toHaveBeenCalledTimes(1); + expect(api).toHaveBeenCalledExactlyOnceWith( + '/spaces/space/pages/page-a/conversation', + 'POST', + { dotId: dot.id }, + ); + expect(root.root.findByType(Chat).props.thread).toEqual(thread); + expect(p.onOpenChange).toHaveBeenCalledWith(true); + expect(p.onRefresh).toHaveBeenCalledTimes(1); + }, +); + +it.each(['save', 'api'])( + 'releases busy state and permits retry after a %s failure', + async (failure) => { + const p = props(); + if (failure === 'save') p.beforeChat.mockResolvedValueOnce(false); + else api.mockRejectedValueOnce(new Error('Conversation unavailable.')); + const root = await mount(p); + await submit(root, 'Explain this page'); + expect(root.root.findByProps({ role: 'alert' }).props.children).toContain( + failure === 'save' ? 'Save or resolve' : 'Conversation unavailable.', + ); + expect(api).toHaveBeenCalledTimes(failure === 'save' ? 0 : 1); + expect(sendButton(root).props.disabled).toBe(false); + expect(root.root.findByType('input').props.disabled).toBe(false); + await submit(root); + expect(root.root.findByType(Chat).props.initialPrompt).toBe( + 'Explain this page', + ); + expect(p.onRefresh).toHaveBeenCalledTimes(1); + }, +); + +it.each([ + ['page', 'success'], + ['page', 'error'], + ['dot', 'success'], + ['dot', 'error'], +])( + 'ignores an old %s chat %s without settling a newer request', + async (change, outcome) => { + const p = props(); + const old = deferred(), + latest = deferred(); + api.mockReturnValueOnce(old.promise).mockReturnValueOnce(latest.promise); + const root = await mount(p); + await submit(root, 'Explain this page'); + expect(api).toHaveBeenCalledTimes(1); + const next = { + ...p, + ...(change === 'page' + ? { page: { ...p.page, id: 'page-b' } } + : { workspace: { ...p.workspace, dots: [{ ...dot, id: 'dot-b' }] } }), + }; + await act(async () => root.update(view(next))); + await submit(root); + expect(api).toHaveBeenCalledTimes(2); + await act(async () => { + if (outcome === 'success') old.resolve(thread); + else old.reject(new Error('Stale failure.')); + }); + expect(p.onRefresh).not.toHaveBeenCalled(); + expect(p.onOpenChange).not.toHaveBeenCalledWith(true); + expect(root.root.findAllByProps({ role: 'alert' })).toHaveLength(0); + expect(sendButton(root).props.disabled).toBe(true); + const nextThread = { + ...thread, + id: 'thread-b', + dotId: next.workspace.dots[0].id, + }; + await act(async () => latest.resolve(nextThread)); + expect(root.root.findByType(Chat).props.thread).toEqual(nextThread); + expect(p.onRefresh).toHaveBeenCalledTimes(1); + }, +); + +it.each(['success', 'error'])( + 'ignores a chat %s that arrives after unmount', + async (outcome) => { + const p = props(); + const pending = deferred(); + api.mockReturnValueOnce(pending.promise); + const root = await mount(p); + await submit(root, 'Explain this page'); + expect(api).toHaveBeenCalledTimes(1); + await act(async () => root.unmount()); + p.onOpenChange.mockClear(); + await act(async () => { + if (outcome === 'success') pending.resolve(thread); + else pending.reject(new Error('Stale failure.')); + }); + expect(p.onRefresh).not.toHaveBeenCalled(); + expect(p.onOpenChange).not.toHaveBeenCalled(); + }, +);