diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index 7e1963c5..bc367d8d 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -10,6 +10,7 @@ import SharedAgentGate from './agents/SharedAgentGate'; import DocsGPT3 from './assets/cute_docsgpt3.svg'; import ActionButtons from './components/ActionButtons'; import AdminRoute from './components/AdminRoute'; +import ErrorBoundary from './components/ErrorBoundary'; import Spinner from './components/Spinner'; import { Button } from './components/ui/button'; import UploadToast from './components/UploadToast'; @@ -75,6 +76,7 @@ function AuthWrapper({ children }: { children: React.ReactNode }) { function MainLayout() { const { isMobile, isTablet } = useMediaQuery(); const [navOpen, setNavOpen] = useState(!(isMobile || isTablet)); + const location = useLocation(); return (
@@ -87,7 +89,11 @@ function MainLayout() { : 'ml-0 lg:ml-16' }`} > - + {/* Contain route render crashes so navigation stays usable; + keyed by path so the boundary resets when the user leaves. */} + + +
diff --git a/frontend/src/components/ErrorBoundary.test.tsx b/frontend/src/components/ErrorBoundary.test.tsx new file mode 100644 index 00000000..6a34a0ea --- /dev/null +++ b/frontend/src/components/ErrorBoundary.test.tsx @@ -0,0 +1,102 @@ +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; + +vi.mock('react-i18next', () => ({ + useTranslation: () => ({ t: (key: string) => key }), +})); + +import ErrorBoundary from './ErrorBoundary'; + +Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); + +function Bomb({ armed }: { armed: boolean }) { + if (armed) throw new Error('render exploded'); + return
safe child
; +} + +describe('ErrorBoundary', () => { + let container: HTMLDivElement; + let root: Root; + let consoleError: ReturnType; + + beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + // React re-logs caught render errors; keep test output clean. + consoleError = vi.spyOn(console, 'error').mockImplementation(() => {}); + }); + + afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); + consoleError.mockRestore(); + }); + + it('renders children when nothing throws', async () => { + await act(async () => { + root.render( + + + , + ); + }); + expect(container.querySelector('[data-testid="child"]')).not.toBeNull(); + }); + + it('contains a throwing child and shows the fallback', async () => { + await act(async () => { + root.render( + + + , + ); + }); + expect(container.querySelector('[data-testid="child"]')).toBeNull(); + expect(container.querySelector('[role="alert"]')).not.toBeNull(); + expect(consoleError).toHaveBeenCalled(); + }); + + it('recovers via the retry button once the cause is gone', async () => { + await act(async () => { + root.render( + + + , + ); + }); + // The cause disappears (e.g. new data), but the boundary still holds. + await act(async () => { + root.render( + + + , + ); + }); + expect(container.querySelector('[role="alert"]')).not.toBeNull(); + + const button = container.querySelector('button'); + expect(button).not.toBeNull(); + await act(async () => { + button!.dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); + expect(container.querySelector('[data-testid="child"]')).not.toBeNull(); + }); + + it('uses a custom fallback render prop when provided', async () => { + await act(async () => { + root.render( + ( + + )} + > + + , + ); + }); + expect(container.querySelector('[data-testid="custom"]')).not.toBeNull(); + }); +}); diff --git a/frontend/src/components/ErrorBoundary.tsx b/frontend/src/components/ErrorBoundary.tsx new file mode 100644 index 00000000..c5de95a6 --- /dev/null +++ b/frontend/src/components/ErrorBoundary.tsx @@ -0,0 +1,57 @@ +import { Component, type ErrorInfo, type ReactNode } from 'react'; +import { useTranslation } from 'react-i18next'; + +import { Button } from './ui/button'; + +type ErrorBoundaryProps = { + children: ReactNode; + // Render-prop fallback; receives a retry callback that re-attempts + // rendering the children. + fallback?: (retry: () => void) => ReactNode; +}; + +type ErrorBoundaryState = { + hasError: boolean; +}; + +function DefaultFallback({ onRetry }: { onRetry: () => void }) { + const { t } = useTranslation(); + return ( +
+

{t('errorBoundary.message')}

+ +
+ ); +} + +export default class ErrorBoundary extends Component< + ErrorBoundaryProps, + ErrorBoundaryState +> { + state: ErrorBoundaryState = { hasError: false }; + + static getDerivedStateFromError(): ErrorBoundaryState { + return { hasError: true }; + } + + componentDidCatch(error: Error, info: ErrorInfo) { + console.error('ErrorBoundary caught a render error:', error, info); + } + + retry = () => { + this.setState({ hasError: false }); + }; + + render() { + if (this.state.hasError) { + if (this.props.fallback) return this.props.fallback(this.retry); + return ; + } + return this.props.children; + } +} diff --git a/frontend/src/components/MessageInput.tsx b/frontend/src/components/MessageInput.tsx index 3c396f06..a35aa9a1 100644 --- a/frontend/src/components/MessageInput.tsx +++ b/frontend/src/components/MessageInput.tsx @@ -44,6 +44,7 @@ import { SourcesTrigger, ToolsTrigger, } from './message-input'; +import { useArmedSend } from './message-input/armedSend'; import { handleAbort } from '../conversation/conversationSlice'; import { AUDIO_FILE_ACCEPT_ATTR, @@ -1483,34 +1484,52 @@ export default function MessageInput({ }; // When ``allowSendWithoutText`` is set, an attachment-only submit is - // permitted as long as at least one attachment has finished processing. - const hasCompletedAttachment = attachments.some( + // permitted as long as at least one attachment exists; a still-pending + // one arms the send instead of submitting (see handleSubmit). + const completedAttachmentCount = attachments.filter( (attachment) => attachment.status === 'completed', - ); + ).length; const hasSubmittableContent = - Boolean(value.trim()) || (allowSendWithoutText && hasCompletedAttachment); + Boolean(value.trim()) || (allowSendWithoutText && attachments.length > 0); const canSubmit = hasSubmittableContent && !loading && recordingState !== 'recording' && recordingState !== 'transcribing'; - const handleSubmit = () => { - if (canSubmit) { - onSubmit(value); - setValue(''); - if (isTouch) { - inputRef.current?.blur(); - } else if (autoFocus) { - setTimeout(() => { - if (isMountedRef.current) { - inputRef.current?.focus(); - } - }, 0); - } + const submitNow = () => { + onSubmit(value); + setValue(''); + if (isTouch) { + inputRef.current?.blur(); + } else if (autoFocus) { + setTimeout(() => { + if (isMountedRef.current) { + inputRef.current?.focus(); + } + }, 0); } }; + const { + armed: sendArmed, + readiness: sendReadiness, + arm: armSend, + cancel: cancelArmedSend, + } = useArmedSend({ attachments, onFlush: submitNow }); + + const handleSubmit = () => { + if (!canSubmit) return; + // Attachments still uploading/parsing (or failed) must never be + // silently dropped from the payload: hold the send in the composer + // until every attachment resolves, then flush automatically. + if (sendReadiness.state !== 'ready') { + armSend(); + return; + } + submitNow(); + }; + const handleCancel = () => { handleAbort(); }; @@ -1571,6 +1590,42 @@ export default function MessageInput({ onDropOn={handleDropOn} /> + {sendArmed && sendReadiness.state === 'waiting' && ( +
+ + {t('conversation.attachments.waitingToSend', { + count: sendReadiness.pendingCount, + })} + + +
+ )} + {sendArmed && sendReadiness.state === 'blocked' && ( +
+ {t('conversation.attachments.sendBlockedByFailed', { + names: sendReadiness.failedNames.join(', '), + })} +
+ )} + {!sendArmed && completedAttachmentCount > 0 && ( +
+ {t('conversation.attachments.attachedCount', { + count: completedAttachmentCount, + })} +
+ )} {voiceError && (
{voiceError} diff --git a/frontend/src/components/message-input/armedSend.test.tsx b/frontend/src/components/message-input/armedSend.test.tsx new file mode 100644 index 00000000..f1f602a3 --- /dev/null +++ b/frontend/src/components/message-input/armedSend.test.tsx @@ -0,0 +1,163 @@ +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; + +import type { Attachment } from '../../upload/uploadSlice'; +import { getSendReadiness, useArmedSend } from './armedSend'; + +Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); + +const att = (over: Partial = {}): Attachment => ({ + id: 'a1', + fileName: 'report.pdf', + progress: 100, + status: 'completed', + taskId: 't1', + ...over, +}); + +describe('getSendReadiness', () => { + it('is ready with no attachments', () => { + expect(getSendReadiness([])).toEqual({ state: 'ready' }); + }); + + it('is ready when every attachment is completed', () => { + expect(getSendReadiness([att(), att({ id: 'a2' })])).toEqual({ + state: 'ready', + }); + }); + + it('waits while any attachment is uploading or processing', () => { + expect( + getSendReadiness([ + att(), + att({ id: 'a2', status: 'uploading', progress: 5 }), + att({ id: 'a3', status: 'processing', progress: 40 }), + ]), + ).toEqual({ state: 'waiting', pendingCount: 2 }); + }); + + it('blocks on a failed attachment, listing its name', () => { + expect( + getSendReadiness([ + att(), + att({ id: 'a2', status: 'failed', fileName: 'broken.pdf' }), + ]), + ).toEqual({ state: 'blocked', failedNames: ['broken.pdf'] }); + }); + + it('failed takes precedence over pending', () => { + expect( + getSendReadiness([ + att({ status: 'processing' }), + att({ id: 'a2', status: 'failed', fileName: 'broken.pdf' }), + ]), + ).toEqual({ state: 'blocked', failedNames: ['broken.pdf'] }); + }); +}); + +type HookApi = ReturnType; + +function Host({ + attachments, + onFlush, + api, +}: { + attachments: Attachment[]; + onFlush: () => void; + api: { current: HookApi | null }; +}) { + const hook = useArmedSend({ attachments, onFlush }); + api.current = hook; + return null; +} + +describe('useArmedSend', () => { + let container: HTMLDivElement; + let root: Root; + let api: { current: HookApi | null }; + let onFlush: ReturnType void>>; + + beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + api = { current: null }; + onFlush = vi.fn<() => void>(); + }); + + afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); + }); + + const render = async (attachments: Attachment[]) => { + await act(async () => { + root.render( + , + ); + }); + }; + + it('arms without flushing while attachments are pending', async () => { + await render([att({ status: 'processing', progress: 30 })]); + await act(async () => api.current!.arm()); + + expect(api.current!.armed).toBe(true); + expect(api.current!.readiness.state).toBe('waiting'); + expect(onFlush).not.toHaveBeenCalled(); + }); + + it('auto-flushes exactly once when all attachments complete', async () => { + await render([att({ status: 'processing', progress: 30 })]); + await act(async () => api.current!.arm()); + + await render([att()]); + expect(onFlush).toHaveBeenCalledTimes(1); + expect(api.current!.armed).toBe(false); + + // A later re-render with the same resolved attachments must not re-fire. + await render([att()]); + expect(onFlush).toHaveBeenCalledTimes(1); + }); + + it('holds the flush while a file is failed and resumes when it is removed', async () => { + await render([att({ status: 'processing' })]); + await act(async () => api.current!.arm()); + + await render([att({ status: 'failed', fileName: 'broken.pdf' })]); + expect(onFlush).not.toHaveBeenCalled(); + expect(api.current!.armed).toBe(true); + expect(api.current!.readiness).toEqual({ + state: 'blocked', + failedNames: ['broken.pdf'], + }); + + await render([]); + expect(onFlush).toHaveBeenCalledTimes(1); + expect(api.current!.armed).toBe(false); + }); + + it('cancel disarms and prevents the auto-flush', async () => { + await render([att({ status: 'uploading', progress: 10 })]); + await act(async () => api.current!.arm()); + await act(async () => api.current!.cancel()); + + expect(api.current!.armed).toBe(false); + + await render([att()]); + expect(onFlush).not.toHaveBeenCalled(); + }); + + it('flush uses the latest onFlush callback', async () => { + await render([att({ status: 'processing' })]); + await act(async () => api.current!.arm()); + + const lateFlush = vi.fn(); + await act(async () => { + root.render(); + }); + + expect(onFlush).not.toHaveBeenCalled(); + expect(lateFlush).toHaveBeenCalledTimes(1); + }); +}); diff --git a/frontend/src/components/message-input/armedSend.ts b/frontend/src/components/message-input/armedSend.ts new file mode 100644 index 00000000..6c64d711 --- /dev/null +++ b/frontend/src/components/message-input/armedSend.ts @@ -0,0 +1,56 @@ +import { useCallback, useEffect, useRef, useState } from 'react'; + +import type { Attachment } from '../../upload/uploadSlice'; + +export type SendReadiness = + | { state: 'ready' } + | { state: 'waiting'; pendingCount: number } + | { state: 'blocked'; failedNames: string[] }; + +export function getSendReadiness(attachments: Attachment[]): SendReadiness { + const failedNames = attachments + .filter((a) => a.status === 'failed') + .map((a) => a.fileName); + if (failedNames.length > 0) return { state: 'blocked', failedNames }; + + const pendingCount = attachments.filter( + (a) => a.status === 'uploading' || a.status === 'processing', + ).length; + if (pendingCount > 0) return { state: 'waiting', pendingCount }; + + return { state: 'ready' }; +} + +export function useArmedSend({ + attachments, + onFlush, +}: { + attachments: Attachment[]; + onFlush: () => void; +}) { + const [armed, setArmed] = useState(false); + // Latest-closure ref so the flush submits the current composer value, + // not the one captured when the send was armed. + const flushRef = useRef(onFlush); + flushRef.current = onFlush; + const flushedRef = useRef(false); + + const readiness = getSendReadiness(attachments); + + useEffect(() => { + if (!armed) { + flushedRef.current = false; + return; + } + if (readiness.state === 'ready' && !flushedRef.current) { + flushedRef.current = true; + setArmed(false); + flushRef.current(); + } + }, [armed, readiness.state]); + + const arm = useCallback(() => setArmed(true), []); + const cancel = useCallback(() => setArmed(false), []); + + return { armed, readiness, arm, cancel }; +} diff --git a/frontend/src/conversation/Conversation.tsx b/frontend/src/conversation/Conversation.tsx index 1a532eca..9d2604c2 100644 --- a/frontend/src/conversation/Conversation.tsx +++ b/frontend/src/conversation/Conversation.tsx @@ -7,6 +7,7 @@ import userService from '../api/services/userService'; import SharedAgentCard from '../agents/SharedAgentCard'; import { Agent } from '../agents/types'; import ArtifactSidebar from '../components/ArtifactSidebar'; +import ErrorBoundary from '../components/ErrorBoundary'; import MessageInput from '../components/MessageInput'; import { useMediaQuery } from '../hooks'; import { @@ -31,7 +32,10 @@ import { submitToolActions, updateQuery, } from './conversationSlice'; -import { selectCompletedAttachments } from '../upload/uploadSlice'; +import { + clearAttachments, + selectCompletedAttachments, +} from '../upload/uploadSlice'; export default function Conversation() { const { t } = useTranslation(); @@ -165,8 +169,16 @@ export default function Conversation() { }, [conversationId, status]); const handleFetchAnswer = useCallback( - ({ question, index }: { question: string; index?: number }) => { - dispatch(fetchAnswer({ question, indx: index })); + ({ + question, + index, + attachmentIds, + }: { + question: string; + index?: number; + attachmentIds?: string[]; + }) => { + dispatch(fetchAnswer({ question, indx: index, attachmentIds })); }, [dispatch, selectedAgent], ); @@ -184,11 +196,12 @@ export default function Conversation() { const trimmedQuestion = question.trim(); if (trimmedQuestion === '') return; - const filesAttached = completedAttachments - .filter((a) => a.id) - .map((a) => ({ id: a.id as string, fileName: a.fileName })); - if (index !== undefined) { + // Retry/edit of an existing turn: re-send the ids bound to that + // row — the composer slice was consumed by the original send. + const rowAttachmentIds = (queries[index]?.attachments ?? []).map( + (a) => a.id, + ); dispatch( resendQuery({ index, @@ -196,8 +209,16 @@ export default function Conversation() { keepIdempotencyKey: isRetry, }), ); - handleFetchAnswer({ question: trimmedQuestion, index }); + handleFetchAnswer({ + question: trimmedQuestion, + index, + attachmentIds: rowAttachmentIds, + }); } else { + const filesAttached = completedAttachments + .filter((a) => a.id) + .map((a) => ({ id: a.id as string, fileName: a.fileName })); + if (!isRetry) dispatch( addQuery({ @@ -205,10 +226,17 @@ export default function Conversation() { attachments: filesAttached, }), ); - handleFetchAnswer({ question: trimmedQuestion, index }); + // One source of truth: the ids on the wire are exactly the ids + // the optimistic row displays. + handleFetchAnswer({ + question: trimmedQuestion, + index, + attachmentIds: filesAttached.map((f) => f.id), + }); + if (filesAttached.length > 0) dispatch(clearAttachments()); } }, - [dispatch, handleFetchAnswer, completedAttachments], + [dispatch, handleFetchAnswer, completedAttachments, queries], ); const handleFeedback = (query: Query, feedback: FEEDBACK, index: number) => { @@ -336,38 +364,41 @@ export default function Conversation() { }`} >
- - - navigate( - selectedAgent.agent_type === 'workflow' - ? `/agents/workflow/edit/${selectedAgent.id}` - : `/agents/edit/${selectedAgent.id}`, - ) - : undefined - } - /> -
- ) : undefined - } - /> + {/* A render crash in the message list must leave the composer + usable; the boundary resets on conversation switch. */} + + + + navigate( + selectedAgent.agent_type === 'workflow' + ? `/agents/workflow/edit/${selectedAgent.id}` + : `/agents/edit/${selectedAgent.id}`, + ) + : undefined + } + /> +
+ ) : undefined + } + /> +
({ + handleFetchAnswer: vi.fn().mockResolvedValue(null), + handleFetchAnswerSteaming: vi.fn().mockResolvedValue(undefined), + handleSubmitToolActions: vi.fn(), + handleV1ChatCompletionStreaming: vi.fn(), +})); + +import { configureStore } from '@reduxjs/toolkit'; + +import uploadReducer, { addAttachment } from '../upload/uploadSlice'; +import { handleFetchAnswer } from './conversationHandlers'; import reducer, { + addQuery, applyMessageTail, + fetchAnswer, raiseNotice, + resendQuery, setConversation, } from './conversationSlice'; @@ -182,3 +196,135 @@ describe('raiseNotice — non-fatal notice', () => { expect(next.queries[0].notice).toBeUndefined(); }); }); + +const completedAtt = { + id: 'srv-1', + fileName: 'a.pdf', + progress: 100, + status: 'completed' as const, + taskId: 't1', +}; +const processingAtt = { + id: 'c-2', + fileName: 'b.pdf', + progress: 40, + status: 'processing' as const, + taskId: 't2', +}; + +const preferenceStub = { + token: 'tok', + selectedDocs: [], + prompt: { id: 'default' }, + chunks: '2', + selectedAgent: null, + selectedModel: null, +}; + +const makeStore = () => + configureStore({ + reducer: { + conversation: reducer, + upload: uploadReducer, + preference: () => preferenceStub, + }, + }); + +describe('fetchAnswer — attachment ids on the wire', () => { + beforeEach(() => { + vi.mocked(handleFetchAnswer) + .mockClear() + .mockResolvedValue(null as never); + }); + + it('sends explicit attachmentIds and leaves composer attachments untouched', async () => { + const store = makeStore(); + store.dispatch(addQuery({ prompt: 'q' })); + store.dispatch(addAttachment(completedAtt)); + + await store.dispatch( + fetchAnswer({ + question: 'q', + indx: 0, + attachmentIds: ['row-1', 'row-2'], + }), + ); + + expect(vi.mocked(handleFetchAnswer).mock.calls[0][8]).toEqual([ + 'row-1', + 'row-2', + ]); + // Explicit ids (e.g. a retry of an existing turn) must not consume + // whatever the composer currently holds. + expect(store.getState().upload.attachments).toHaveLength(1); + }); + + it('falls back to completed composer uploads and clears them when no ids are passed', async () => { + const store = makeStore(); + store.dispatch(addQuery({ prompt: 'q' })); + store.dispatch(addAttachment(completedAtt)); + store.dispatch(addAttachment(processingAtt)); + + await store.dispatch(fetchAnswer({ question: 'q', indx: 0 })); + + expect(vi.mocked(handleFetchAnswer).mock.calls[0][8]).toEqual(['srv-1']); + // clearAttachments keeps in-flight rows. + expect(store.getState().upload.attachments.map((a) => a.id)).toEqual([ + 'c-2', + ]); + }); +}); + +describe('fetchAnswer.rejected', () => { + it('writes the error to the retried row, not the last row', () => { + let state = reducer( + undefined, + setConversation([{ prompt: 'first' }, { prompt: 'second' }]), + ); + state = reducer( + state, + fetchAnswer.rejected(new Error('boom'), 'req-1', { + question: 'first', + indx: 0, + }), + ); + expect(state.status).toBe('failed'); + expect(state.queries[0].error).toBe('Something went wrong'); + expect(state.queries[1].error).toBeUndefined(); + }); + + it('defaults to the last row when no index is given', () => { + let state = reducer( + undefined, + setConversation([{ prompt: 'first' }, { prompt: 'second' }]), + ); + state = reducer( + state, + fetchAnswer.rejected(new Error('boom'), 'req-1', { question: 'second' }), + ); + expect(state.queries[0].error).toBeUndefined(); + expect(state.queries[1].error).toBe('Something went wrong'); + }); +}); + +describe('resendQuery', () => { + it('preserves the attachments bound to the turn', () => { + let state = reducer( + undefined, + setConversation([ + { + prompt: 'p', + response: 'r', + error: 'e', + attachments: [{ id: 'x', fileName: 'f.pdf' }], + }, + ]), + ); + state = reducer(state, resendQuery({ index: 0, prompt: 'p2' })); + expect(state.queries[0].attachments).toEqual([ + { id: 'x', fileName: 'f.pdf' }, + ]); + expect(state.queries[0].response).toBeUndefined(); + expect(state.queries[0].error).toBeUndefined(); + }); +}); diff --git a/frontend/src/conversation/conversationSlice.ts b/frontend/src/conversation/conversationSlice.ts index 2fa72501..9e1c95b6 100644 --- a/frontend/src/conversation/conversationSlice.ts +++ b/frontend/src/conversation/conversationSlice.ts @@ -160,20 +160,29 @@ export const loadConversation = createAsyncThunk< export const fetchAnswer = createAsyncThunk< Answer, - { question: string; indx?: number } ->('fetchAnswer', async ({ question, indx }, { dispatch, getState }) => { + { question: string; indx?: number; attachmentIds?: string[] } +>('fetchAnswer', async (arg, { dispatch, getState }) => { + const { question, indx } = arg; if (abortController) abortController.abort(); abortController = new AbortController(); const { signal } = abortController; let isSourceUpdated = false; const state = getState() as RootState; - const attachmentIds = selectCompletedAttachments(state) - .filter((a) => a.id) - .map((a) => a.id) as string[]; - - if (attachmentIds.length > 0) { - dispatch(clearAttachments()); + // Explicit ids (new sends and retries pass the ids bound to the turn) + // keep the wire payload in sync with what the query row displays. + // The fallback reads the composer state for callers that predate the + // argument and consumes it, which loses the ids on a later retry. + let attachmentIds: string[]; + if (arg.attachmentIds !== undefined) { + attachmentIds = arg.attachmentIds; + } else { + attachmentIds = selectCompletedAttachments(state) + .filter((a) => a.id) + .map((a) => a.id) as string[]; + if (attachmentIds.length > 0) { + dispatch(clearAttachments()); + } } // Mutable so the SSE handler can adopt a server-assigned id and @@ -1147,8 +1156,14 @@ export const conversationSlice = createSlice({ } state.status = 'failed'; if (state.queries.length > 0) { - state.queries[state.queries.length - 1].error = - 'Something went wrong'; + // Indexed sends (edit/retry) must fail on their own row, not + // whatever happens to be last. + const indx = action.meta.arg?.indx; + const targetIndex = + indx !== undefined && indx >= 0 && indx < state.queries.length + ? indx + : state.queries.length - 1; + state.queries[targetIndex].error = 'Something went wrong'; } }); }, diff --git a/frontend/src/locale/en.json b/frontend/src/locale/en.json index 064898c6..dd70bb88 100644 --- a/frontend/src/locale/en.json +++ b/frontend/src/locale/en.json @@ -8,6 +8,10 @@ "sourceDocs": "Source", "none": "None", "cancel": "Cancel", + "errorBoundary": { + "message": "Something went wrong displaying this content.", + "tryAgain": "Try again" + }, "help": "Help", "emailUs": "Email us", "documentation": "Documentation", @@ -1090,7 +1094,13 @@ }, "attachments": { "attach": "Attach", - "remove": "Remove attachment" + "remove": "Remove attachment", + "waitingToSend_one": "Will send when {{count}} file finishes processing…", + "waitingToSend_other": "Will send when {{count}} files finish processing…", + "cancelQueuedSend": "Cancel", + "sendBlockedByFailed": "{{names}} couldn't be processed — remove to send", + "attachedCount_one": "Attached: {{count}} file", + "attachedCount_other": "Attached: {{count}} files" }, "retry": "Retry", "reasoning": "Reasoning", diff --git a/tests/e2e/specs/tier-a/composer-resilience.spec.ts b/tests/e2e/specs/tier-a/composer-resilience.spec.ts new file mode 100644 index 00000000..c3f343f5 --- /dev/null +++ b/tests/e2e/specs/tier-a/composer-resilience.spec.ts @@ -0,0 +1,198 @@ +/** + * Composer resilience: the composer must never silently discard a file + * or a send. + * + * Two silent-failure paths pinned here: + * + * 1. Stream POST failure. A failed /stream request must surface a + * visible error bubble with a working Retry — never a send that + * evaporates. (fetchAnswer.rejected → per-row error → Retry.) + * + * 2. Send while an attachment is still uploading/parsing. The composer + * "arms" the send (visible "Will send when … " banner), holds the + * message, and auto-flushes once the attachment completes — so the + * file arrives bound to the turn (`conversation_messages.attachments[]`) + * instead of being silently dropped from the payload. + * + * UI-driven on purpose: the surface under test IS the composer. The + * attachments spec avoids the file picker for API-direct upload tests; + * here `setInputFiles` on the dropzone input is the point. + */ + +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; + +import * as playwright from '@playwright/test'; +const { expect, test } = playwright; + +import type { Page } from '@playwright/test'; + +import { newUserContext } from '../../helpers/auth.js'; +import { pg } from '../../helpers/db.js'; +import { resetDb } from '../../helpers/reset.js'; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const FIXTURES_DIR = resolve(HERE, '..', '..', 'fixtures', 'docs'); +const SMALL_FIXTURE_PATH = resolve(FIXTURES_DIR, 'notes.txt'); + +async function openComposer(page: Page): Promise { + await page.goto('/'); + await expect(page.locator('#message-input')).toBeVisible(); +} + +test.describe('tier-a · composer resilience', () => { + test.beforeEach(async () => { + await resetDb(); + }); + + test('failed /stream surfaces a visible error and Retry re-sends successfully', async ({ + browser, + }) => { + const { context, sub } = await newUserContext(browser); + try { + const page = await context.newPage(); + await openComposer(page); + + // Force the stream POST to die at the network layer — the shape of + // an outage the frontend cannot see an HTTP status for. + await page.route('**/stream', (route) => route.abort('failed')); + + const textarea = page.locator('#message-input'); + await textarea.fill('resilience probe — e2e-composer-retry'); + await textarea.press('Enter'); + + // Visible failure, not a silent drop: the error bubble and a + // usable Retry control. + await expect(page.getByText('Something went wrong')).toBeVisible({ + timeout: 15_000, + }); + const retryButton = page.getByRole('button', { name: /retry/i }); + await expect(retryButton).toBeEnabled(); + + // Network heals; Retry must complete the SAME turn. + await page.unroute('**/stream'); + const streamDone = page.waitForResponse( + (r) => r.url().includes('/stream') && r.request().method() === 'POST', + { timeout: 45_000 }, + ); + await retryButton.click(); + const streamRes = await streamDone; + expect(streamRes.status()).toBe(200); + + await expect(page.getByText('Something went wrong')).toBeHidden({ + timeout: 15_000, + }); + + // The turn landed: one message row, prompt AND response populated. + const { rows } = await pg.query<{ + prompt: string | null; + response: string | null; + }>( + `SELECT cm.prompt, cm.response + FROM conversation_messages cm + JOIN conversations c ON c.id = cm.conversation_id + WHERE c.user_id = $1`, + [sub], + ); + expect(rows).toHaveLength(1); + expect(rows[0].prompt).toBe('resilience probe — e2e-composer-retry'); + expect(rows[0].response).toBeTruthy(); + } finally { + await context.close(); + } + }); + + test('send while a file is still processing arms, waits, and flushes with the file bound to the turn', async ({ + browser, + }) => { + const { context, sub } = await newUserContext(browser); + try { + const page = await context.newPage(); + await openComposer(page); + + // Track /stream POSTs so we can prove the send was HELD. + const streamPosts: string[] = []; + page.on('request', (req) => { + if (req.url().includes('/stream') && req.method() === 'POST') { + streamPosts.push(req.postData() ?? ''); + } + }); + + // Hold the upload response long enough to press Enter while the + // attachment is still visibly 'uploading'. + await page.route('**/api/store_attachment', async (route) => { + await new Promise((r) => setTimeout(r, 2_000)); + await route.continue(); + }); + + // The dropzone input is the first file input MessageInput renders + // (the second is the audio-capture input). + await page + .locator('input[type="file"]') + .first() + .setInputFiles(SMALL_FIXTURE_PATH); + + const textarea = page.locator('#message-input'); + await textarea.fill('what is in the attached file? e2e-armed-send'); + await textarea.press('Enter'); + + // The send is armed, not fired: banner visible, no /stream yet, + // and the prompt is still in the composer. + await expect( + page.getByText(/will send when 1 file finishes processing/i), + ).toBeVisible({ timeout: 5_000 }); + expect(streamPosts).toHaveLength(0); + await expect(textarea).toHaveValue( + 'what is in the attached file? e2e-armed-send', + ); + + // Upload completes → Celery parses → attachment.completed lands via + // SSE → the armed send auto-flushes. notes.txt parses in seconds. + const streamDone = page.waitForResponse( + (r) => r.url().includes('/stream') && r.request().method() === 'POST', + { timeout: 60_000 }, + ); + const streamRes = await streamDone; + expect(streamRes.status()).toBe(200); + + // The flushed payload carried exactly one attachment id. + expect(streamPosts).toHaveLength(1); + const payload = JSON.parse(streamPosts[0]) as { + attachments?: string[]; + }; + expect(payload.attachments).toHaveLength(1); + + // Banner cleared, composer emptied by the flush. + await expect( + page.getByText(/will send when/i), + ).toBeHidden({ timeout: 15_000 }); + await expect(textarea).toHaveValue(''); + + // The invariant that motivated all of this: the file is BOUND to + // the turn — conversation_messages.attachments[] holds one PG PK + // that resolves to this user's attachments row. + const { rows } = await pg.query<{ + prompt: string | null; + attachment_count: number | string | null; + resolved: number | string | null; + }>( + `SELECT cm.prompt, + COALESCE(array_length(cm.attachments, 1), 0) AS attachment_count, + (SELECT count(*) FROM attachments a + WHERE a.id = ANY(cm.attachments)) AS resolved + FROM conversation_messages cm + JOIN conversations c ON c.id = cm.conversation_id + WHERE c.user_id = $1`, + [sub], + ); + expect(rows).toHaveLength(1); + expect(rows[0].prompt).toBe( + 'what is in the attached file? e2e-armed-send', + ); + expect(Number(rows[0].attachment_count)).toBe(1); + expect(Number(rows[0].resolved)).toBe(1); + } finally { + await context.close(); + } + }); +});