From a9c0082e9afbeb8589a9767aadc83260fc6433da Mon Sep 17 00:00:00 2001 From: arc53-machine <232052973+arc53-machine@users.noreply.github.com> Date: Mon, 28 Sep 2026 10:05:19 +0100 Subject: [PATCH 1/7] Send with a failed attachment instead of blocking A failed attachment made getSendReadiness return 'blocked', so Send was a no-op until the user found and removed the chip. On a phone the only hint was a small red line after the tap, and a new user lost their question that way. A failed file never recovers, so holding the send only costs the question. Readiness now ignores failed attachments: it waits only on uploading or processing files. On submit the composer drops every failed attachment and sends the text with the completed ones. The payloads already read completed attachments only, so no failed id reaches the request or the history. The hero-card path in Conversation also clears failed chips. The failed chip still shows before Send and can be removed by hand. Removes the send-blocked alert and its locale key. --- frontend/src/components/MessageInput.test.tsx | 139 ++++++++++++++++++ frontend/src/components/MessageInput.tsx | 36 +++-- .../message-input/armedSend.test.tsx | 32 ++-- .../src/components/message-input/armedSend.ts | 11 +- frontend/src/conversation/Conversation.tsx | 10 +- .../conversation/conversationSlice.test.ts | 18 +++ frontend/src/locale/en.json | 1 - 7 files changed, 203 insertions(+), 44 deletions(-) create mode 100644 frontend/src/components/MessageInput.test.tsx diff --git a/frontend/src/components/MessageInput.test.tsx b/frontend/src/components/MessageInput.test.tsx new file mode 100644 index 00000000..2d5e048a --- /dev/null +++ b/frontend/src/components/MessageInput.test.tsx @@ -0,0 +1,139 @@ +import { configureStore } from '@reduxjs/toolkit'; +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { Provider } from 'react-redux'; + +vi.mock('react-i18next', () => ({ + useTranslation: () => ({ t: (key: string) => key }), +})); +// The upload modal pulls in the whole ingest UI; the composer never opens it here. +vi.mock('../upload/Upload', () => ({ default: () => null })); + +import notificationsReducer from '../notifications/notificationsSlice'; +import { prefSlice } from '../preferences/preferenceSlice'; +import type { RootState } from '../store'; +import uploadReducer, { + addAttachment, + selectCompletedAttachments, + type Attachment, +} from '../upload/uploadSlice'; +import MessageInput from './MessageInput'; + +Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); + +const makeStore = () => + configureStore({ + reducer: { + preference: prefSlice.reducer, + upload: uploadReducer, + notifications: notificationsReducer, + }, + }); + +type TestStore = ReturnType; + +const att = (over: Partial = {}): Attachment => ({ + id: 'ok-1', + fileName: 'notes.pdf', + progress: 100, + status: 'completed', + taskId: 't1', + ...over, +}); + +describe('MessageInput send with a failed attachment', () => { + let container: HTMLDivElement; + let root: Root; + let store: TestStore; + let onSubmit: ReturnType void>>; + + beforeEach(() => { + localStorage.clear(); + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + store = makeStore(); + onSubmit = vi.fn<(text: string) => void>(); + }); + + afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); + }); + + const render = async (loading = false) => { + await act(async () => { + root.render( + + + , + ); + }); + }; + + const textarea = () => + container.querySelector('#message-input')!; + + const type = async (text: string) => { + const el = textarea(); + await act(async () => { + const setter = Object.getOwnPropertyDescriptor( + HTMLTextAreaElement.prototype, + 'value', + )!.set!; + setter.call(el, text); + el.dispatchEvent(new Event('input', { bubbles: true })); + }); + }; + + const pressEnter = async () => { + await act(async () => { + textarea().dispatchEvent( + new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }), + ); + }); + }; + + it('sends the question and drops the failed file', async () => { + store.dispatch(addAttachment(att())); + store.dispatch( + addAttachment( + att({ id: 'bad-1', fileName: 'broken.pdf', status: 'failed' }), + ), + ); + let idsAtSubmit: string[] = []; + onSubmit.mockImplementation(() => { + idsAtSubmit = selectCompletedAttachments( + store.getState() as unknown as RootState, + ).map((a) => a.id); + }); + await render(); + + await type('What does the contract say?'); + await pressEnter(); + + expect(onSubmit).toHaveBeenCalledTimes(1); + expect(onSubmit).toHaveBeenCalledWith('What does the contract say?'); + expect(idsAtSubmit).toEqual(['ok-1']); + expect(store.getState().upload.attachments.map((a) => a.id)).toEqual([ + 'ok-1', + ]); + expect(textarea().value).toBe(''); + }); + + it('shows no send-blocked alert after a failed send attempt', async () => { + store.dispatch(addAttachment(att({ id: 'bad-1', status: 'failed' }))); + await render(); + + await type('hello'); + await pressEnter(); + + expect(container.querySelector('[role="alert"]')).toBeNull(); + }); +}); diff --git a/frontend/src/components/MessageInput.tsx b/frontend/src/components/MessageInput.tsx index c9a90216..1a5327d6 100644 --- a/frontend/src/components/MessageInput.tsx +++ b/frontend/src/components/MessageInput.tsx @@ -1578,10 +1578,12 @@ export default function MessageInput({ }; // When ``allowSendWithoutText`` is set, an attachment-only submit is - // permitted as long as at least one attachment exists; a still-pending - // one arms the send instead of submitting (see handleSubmit). + // permitted as long as at least one attachment can still go out; a + // still-pending one arms the send instead of submitting (see handleSubmit). + // A failed one doesn't count: it is dropped at submit time. const hasSubmittableContent = - Boolean(value.trim()) || (allowSendWithoutText && attachments.length > 0); + Boolean(value.trim()) || + (allowSendWithoutText && attachments.some((a) => a.status !== 'failed')); const canSubmit = hasSubmittableContent && !loading && @@ -1589,6 +1591,17 @@ export default function MessageInput({ recordingState !== 'transcribing'; const submitNow = () => { + const failed = attachments.filter((a) => a.status === 'failed'); + const hasContent = + Boolean(value.trim()) || + (allowSendWithoutText && + attachments.some((a) => a.status === 'completed')); + // An attachment-only send whose files all failed has nothing left to + // send; keep the failed chips so the user can see why. + if (!hasContent) return; + // A failed file will never succeed, so it must never cost the user their + // question: drop it and send with whatever did upload. + failed.forEach((a) => dispatch(removeAttachment(a.id))); onSubmit(value); setValue(''); if (isTouch) { @@ -1621,9 +1634,10 @@ export default function MessageInput({ 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. + // Attachments still uploading/parsing must never be silently dropped + // from the payload: hold the send in the composer until every + // attachment resolves, then flush automatically. Failed ones don't hold + // it; submitNow drops them. if (sendReadiness.state !== 'ready') { armSend(); return; @@ -1722,16 +1736,6 @@ export default function MessageInput({ )} - {sendArmed && sendReadiness.state === 'blocked' && ( -
- {t('conversation.attachments.sendBlockedByFailed', { - names: sendReadiness.failedNames.join(', '), - })} -
- )} {voiceError && (
{voiceError} diff --git a/frontend/src/components/message-input/armedSend.test.tsx b/frontend/src/components/message-input/armedSend.test.tsx index f1f602a3..519b5030 100644 --- a/frontend/src/components/message-input/armedSend.test.tsx +++ b/frontend/src/components/message-input/armedSend.test.tsx @@ -36,22 +36,28 @@ describe('getSendReadiness', () => { ).toEqual({ state: 'waiting', pendingCount: 2 }); }); - it('blocks on a failed attachment, listing its name', () => { + it('ignores a failed attachment: it is dropped at send time', () => { expect( getSendReadiness([ att(), att({ id: 'a2', status: 'failed', fileName: 'broken.pdf' }), ]), - ).toEqual({ state: 'blocked', failedNames: ['broken.pdf'] }); + ).toEqual({ state: 'ready' }); }); - it('failed takes precedence over pending', () => { + it('is ready when every attachment failed', () => { + expect(getSendReadiness([att({ status: 'failed' })])).toEqual({ + state: 'ready', + }); + }); + + it('still waits on a pending file next to a failed one', () => { expect( getSendReadiness([ - att({ status: 'processing' }), + att({ status: 'uploading', progress: 5 }), att({ id: 'a2', status: 'failed', fileName: 'broken.pdf' }), ]), - ).toEqual({ state: 'blocked', failedNames: ['broken.pdf'] }); + ).toEqual({ state: 'waiting', pendingCount: 1 }); }); }); @@ -120,21 +126,17 @@ describe('useArmedSend', () => { 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' })]); + it('flushes once when the pending file fails instead of completing', async () => { + await render([att({ status: 'uploading', progress: 5 })]); await act(async () => api.current!.arm()); + expect(onFlush).not.toHaveBeenCalled(); 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); + + await render([att({ status: 'failed', fileName: 'broken.pdf' })]); + expect(onFlush).toHaveBeenCalledTimes(1); }); it('cancel disarms and prevents the auto-flush', async () => { diff --git a/frontend/src/components/message-input/armedSend.ts b/frontend/src/components/message-input/armedSend.ts index 6c64d711..34d8ef75 100644 --- a/frontend/src/components/message-input/armedSend.ts +++ b/frontend/src/components/message-input/armedSend.ts @@ -3,16 +3,11 @@ 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[] }; + { state: 'ready' } | { state: 'waiting'; pendingCount: number }; +// A failed file never resolves, so it never holds the send: the composer +// drops it at submit time and sends the question with whatever succeeded. 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; diff --git a/frontend/src/conversation/Conversation.tsx b/frontend/src/conversation/Conversation.tsx index ee06afda..0679c160 100644 --- a/frontend/src/conversation/Conversation.tsx +++ b/frontend/src/conversation/Conversation.tsx @@ -224,9 +224,9 @@ export default function Conversation() { }); } else if (getSendReadiness(attachments).state !== 'ready') { // Direct new sends (hero suggestion cards) bypass MessageInput's - // submit gate. With files still uploading/parsing (or failed), - // sending now would silently drop them — route the question into - // the composer instead, where the armed-send banner takes over. + // submit gate. With files still uploading/parsing, sending now + // would silently drop them — route the question into the composer + // instead, where the armed-send banner takes over. setQueuedQuestion(trimmedQuestion); } else { const filesAttached = completedAttachments @@ -247,7 +247,9 @@ export default function Conversation() { index, attachmentIds: filesAttached.map((f) => f.id), }); - if (filesAttached.length > 0) dispatch(clearAttachments()); + // Clears the sent files and drops any failed ones, which never + // hold a send (nothing is pending here, so nothing else remains). + if (attachments.length > 0) dispatch(clearAttachments()); } }, [dispatch, handleFetchAnswer, completedAttachments, attachments, queries], diff --git a/frontend/src/conversation/conversationSlice.test.ts b/frontend/src/conversation/conversationSlice.test.ts index 7ceeb0d4..47fab4cc 100644 --- a/frontend/src/conversation/conversationSlice.test.ts +++ b/frontend/src/conversation/conversationSlice.test.ts @@ -274,6 +274,24 @@ describe('fetchAnswer — attachment ids on the wire', () => { 'c-2', ]); }); + it('leaves a failed composer upload out of the fallback ids', async () => { + const store = makeStore(); + store.dispatch(addQuery({ prompt: 'q' })); + store.dispatch(addAttachment(completedAtt)); + store.dispatch( + addAttachment({ + id: 'f-3', + fileName: 'broken.pdf', + progress: 0, + status: 'failed', + taskId: '', + }), + ); + + await store.dispatch(fetchAnswer({ question: 'q', indx: 0 })); + + expect(vi.mocked(handleFetchAnswer).mock.calls[0][8]).toEqual(['srv-1']); + }); }); describe('fetchAnswer.rejected', () => { diff --git a/frontend/src/locale/en.json b/frontend/src/locale/en.json index 1d3e4a2a..fa295e00 100644 --- a/frontend/src/locale/en.json +++ b/frontend/src/locale/en.json @@ -1382,7 +1382,6 @@ "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", "unsupportedType": "Not a supported file type ({{extension}})", "unreadableByModel": "{{model}} can't read {{name}} — switch to a model that supports images or PDFs", "attached": "Attachment", From be6fe8d0c2439a1cd76cbaedb177e6e3ea5ab4b6 Mon Sep 17 00:00:00 2001 From: arc53-machine <232052973+arc53-machine@users.noreply.github.com> Date: Mon, 28 Sep 2026 10:05:39 +0100 Subject: [PATCH 2/7] Hold a queued send while an answer is streaming A queued send flushed by calling submitNow directly, which skipped the loading check in canSubmit. submitNow clears the composer before the consumer runs, and the consumer ignores a submit while status is 'loading'. If a retry or an edit was streaming when the attachments settled, the question was cleared and never sent. useArmedSend takes a canFlush flag. The composer passes whether it can take a submit right now (not loading, not recording or transcribing). The armed send stays queued until that holds, then goes out. --- frontend/src/components/MessageInput.test.tsx | 25 +++++++++++++++++ frontend/src/components/MessageInput.tsx | 13 +++++++-- .../message-input/armedSend.test.tsx | 28 +++++++++++++++++-- .../src/components/message-input/armedSend.ts | 8 ++++-- 4 files changed, 66 insertions(+), 8 deletions(-) diff --git a/frontend/src/components/MessageInput.test.tsx b/frontend/src/components/MessageInput.test.tsx index 2d5e048a..ca88fdf0 100644 --- a/frontend/src/components/MessageInput.test.tsx +++ b/frontend/src/components/MessageInput.test.tsx @@ -136,4 +136,29 @@ describe('MessageInput send with a failed attachment', () => { expect(container.querySelector('[role="alert"]')).toBeNull(); }); + + it('keeps a queued question while another answer is streaming', async () => { + store.dispatch(addAttachment(att({ status: 'processing', progress: 30 }))); + await render(); + await type('Queued question'); + await pressEnter(); + expect(onSubmit).not.toHaveBeenCalled(); + + // A retry starts streaming, then the file settles. + await render(true); + await act(async () => { + store.dispatch({ + type: 'upload/updateAttachment', + payload: { id: 'ok-1', updates: { status: 'completed' } }, + }); + }); + + expect(onSubmit).not.toHaveBeenCalled(); + expect(textarea().value).toBe('Queued question'); + + // The stream ends: the held send goes out. + await render(false); + expect(onSubmit).toHaveBeenCalledTimes(1); + expect(onSubmit).toHaveBeenCalledWith('Queued question'); + }); }); diff --git a/frontend/src/components/MessageInput.tsx b/frontend/src/components/MessageInput.tsx index 1a5327d6..05920510 100644 --- a/frontend/src/components/MessageInput.tsx +++ b/frontend/src/components/MessageInput.tsx @@ -1584,11 +1584,11 @@ export default function MessageInput({ const hasSubmittableContent = Boolean(value.trim()) || (allowSendWithoutText && attachments.some((a) => a.status !== 'failed')); - const canSubmit = - hasSubmittableContent && + const composerIdle = !loading && recordingState !== 'recording' && recordingState !== 'transcribing'; + const canSubmit = hasSubmittableContent && composerIdle; const submitNow = () => { const failed = attachments.filter((a) => a.status === 'failed'); @@ -1620,7 +1620,14 @@ export default function MessageInput({ readiness: sendReadiness, arm: armSend, cancel: cancelArmedSend, - } = useArmedSend({ attachments, onFlush: submitNow }); + } = useArmedSend({ + attachments, + onFlush: submitNow, + // A queued send that settles while another answer streams would be + // refused by the consumer after the composer was already cleared; hold + // it until the composer can take a submit again. + canFlush: composerIdle, + }); // Adopt a question queued outside the composer: seed the input, arm, // and hand the wait to the standard banner. If the attachments already diff --git a/frontend/src/components/message-input/armedSend.test.tsx b/frontend/src/components/message-input/armedSend.test.tsx index 519b5030..4fe8a928 100644 --- a/frontend/src/components/message-input/armedSend.test.tsx +++ b/frontend/src/components/message-input/armedSend.test.tsx @@ -66,13 +66,15 @@ type HookApi = ReturnType; function Host({ attachments, onFlush, + canFlush, api, }: { attachments: Attachment[]; onFlush: () => void; + canFlush?: boolean; api: { current: HookApi | null }; }) { - const hook = useArmedSend({ attachments, onFlush }); + const hook = useArmedSend({ attachments, onFlush, canFlush }); api.current = hook; return null; } @@ -96,10 +98,15 @@ describe('useArmedSend', () => { container.remove(); }); - const render = async (attachments: Attachment[]) => { + const render = async (attachments: Attachment[], canFlush?: boolean) => { await act(async () => { root.render( - , + , ); }); }; @@ -139,6 +146,21 @@ describe('useArmedSend', () => { expect(onFlush).toHaveBeenCalledTimes(1); }); + it('holds a ready flush until canFlush turns true', async () => { + await render([att({ status: 'processing' })]); + await act(async () => api.current!.arm()); + + // Files resolved, but another answer is streaming: flushing now would + // clear the composer while the submit is refused. + await render([att()], false); + expect(onFlush).not.toHaveBeenCalled(); + expect(api.current!.armed).toBe(true); + + await render([att()], true); + 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()); diff --git a/frontend/src/components/message-input/armedSend.ts b/frontend/src/components/message-input/armedSend.ts index 34d8ef75..49cf712a 100644 --- a/frontend/src/components/message-input/armedSend.ts +++ b/frontend/src/components/message-input/armedSend.ts @@ -19,9 +19,13 @@ export function getSendReadiness(attachments: Attachment[]): SendReadiness { export function useArmedSend({ attachments, onFlush, + canFlush = true, }: { attachments: Attachment[]; onFlush: () => void; + // False while the composer can't take a submit (an answer is streaming): + // the armed send holds until it can, instead of flushing into a refusal. + canFlush?: boolean; }) { const [armed, setArmed] = useState(false); // Latest-closure ref so the flush submits the current composer value, @@ -37,12 +41,12 @@ export function useArmedSend({ flushedRef.current = false; return; } - if (readiness.state === 'ready' && !flushedRef.current) { + if (readiness.state === 'ready' && canFlush && !flushedRef.current) { flushedRef.current = true; setArmed(false); flushRef.current(); } - }, [armed, readiness.state]); + }, [armed, readiness.state, canFlush]); const arm = useCallback(() => setArmed(true), []); const cancel = useCallback(() => setArmed(false), []); From 2acfa34bc514f54261750b2dfe82a1d9647b876a Mon Sep 17 00:00:00 2001 From: arc53-machine <232052973+arc53-machine@users.noreply.github.com> Date: Mon, 28 Sep 2026 10:06:21 +0100 Subject: [PATCH 3/7] Fail stalled attachment uploads and say why an upload failed Uploads had no timeout, so a stalled XHR could leave a chip on "uploading" forever, and a queued send waited with it. Network failures also set no errorMessage, so the failed chip showed no reason. guardUploadStall aborts an upload once it goes 120 s without progress. It restarts on every upload progress event and again when the body finishes sending. A large file on a slow link keeps going; only a silent stall is cut off. That is why it is not xhr.timeout, which caps the whole request against a 100 MB file limit. onerror, onabort and ontimeout now mark the chip failed with "Upload failed. The file couldn't be read or the connection dropped." (single and multi-file paths). A failed chip is dropped at send, so a queued question always goes out in the end. The 5-minute processing cap is unchanged. --- frontend/src/components/MessageInput.test.tsx | 101 ++++++++++++++++++ frontend/src/components/MessageInput.tsx | 56 ++++++---- .../message-input/uploadStallGuard.test.ts | 54 ++++++++++ .../message-input/uploadStallGuard.ts | 33 ++++++ frontend/src/locale/de.json | 1 + frontend/src/locale/en.json | 1 + frontend/src/locale/es.json | 1 + frontend/src/locale/jp.json | 1 + frontend/src/locale/ru.json | 1 + frontend/src/locale/zh-TW.json | 1 + frontend/src/locale/zh.json | 1 + 11 files changed, 232 insertions(+), 19 deletions(-) create mode 100644 frontend/src/components/message-input/uploadStallGuard.test.ts create mode 100644 frontend/src/components/message-input/uploadStallGuard.ts diff --git a/frontend/src/components/MessageInput.test.tsx b/frontend/src/components/MessageInput.test.tsx index ca88fdf0..ff658ed7 100644 --- a/frontend/src/components/MessageInput.test.tsx +++ b/frontend/src/components/MessageInput.test.tsx @@ -18,6 +18,7 @@ import uploadReducer, { type Attachment, } from '../upload/uploadSlice'; import MessageInput from './MessageInput'; +import { UPLOAD_STALL_TIMEOUT_MS } from './message-input/uploadStallGuard'; Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); @@ -41,14 +42,53 @@ const att = (over: Partial = {}): Attachment => ({ ...over, }); +/** Just enough of XMLHttpRequest for the composer's upload paths. */ +class FakeXHR extends EventTarget { + static instances: FakeXHR[] = []; + + upload = new EventTarget(); + status = 0; + responseText = ''; + timeout = 0; + onload: (() => void) | null = null; + onerror: (() => void) | null = null; + onabort: (() => void) | null = null; + ontimeout: (() => void) | null = null; + + open() {} + setRequestHeader() {} + send() { + FakeXHR.instances.push(this); + } + + private finish(handler: (() => void) | null, type: string) { + handler?.(); + this.dispatchEvent(new Event(type)); + this.dispatchEvent(new Event('loadend')); + } + + /** What Android Chrome reported: no response, status 0. */ + failNetwork() { + this.status = 0; + this.finish(this.onerror, 'error'); + } + + abort() { + this.finish(this.onabort, 'abort'); + } +} + describe('MessageInput send with a failed attachment', () => { let container: HTMLDivElement; let root: Root; let store: TestStore; let onSubmit: ReturnType void>>; + const realXHR = globalThis.XMLHttpRequest; beforeEach(() => { localStorage.clear(); + FakeXHR.instances = []; + globalThis.XMLHttpRequest = FakeXHR as unknown as typeof XMLHttpRequest; container = document.createElement('div'); document.body.appendChild(container); root = createRoot(container); @@ -59,6 +99,8 @@ describe('MessageInput send with a failed attachment', () => { afterEach(async () => { await act(async () => root.unmount()); container.remove(); + globalThis.XMLHttpRequest = realXHR; + vi.useRealTimers(); }); const render = async (loading = false) => { @@ -100,6 +142,27 @@ describe('MessageInput send with a failed attachment', () => { }); }; + const attachFile = async (name: string) => { + const input = container.querySelector( + 'label input[type="file"]', + )!; + const file = new File(['%PDF-1.4'], name, { type: 'application/pdf' }); + Object.defineProperty(input, 'files', { + value: [file], + configurable: true, + }); + const sent = FakeXHR.instances.length; + await act(async () => { + input.dispatchEvent(new Event('change', { bubbles: true })); + }); + // partitionAttachmentFiles sniffs the file asynchronously. + await act(async () => { + await vi.waitFor(() => + expect(FakeXHR.instances.length).toBeGreaterThan(sent), + ); + }); + }; + it('sends the question and drops the failed file', async () => { store.dispatch(addAttachment(att())); store.dispatch( @@ -127,6 +190,24 @@ describe('MessageInput send with a failed attachment', () => { expect(textarea().value).toBe(''); }); + it('sends when the only attachment failed on the network', async () => { + await render(); + await attachFile('scan.pdf'); + expect(FakeXHR.instances).toHaveLength(1); + + await act(async () => FakeXHR.instances[0].failNetwork()); + const [chip] = store.getState().upload.attachments; + expect(chip.status).toBe('failed'); + expect(chip.errorMessage).toBe('conversation.attachments.uploadFailed'); + + await type('Summarise this'); + await pressEnter(); + + expect(onSubmit).toHaveBeenCalledTimes(1); + expect(onSubmit).toHaveBeenCalledWith('Summarise this'); + expect(store.getState().upload.attachments).toEqual([]); + }); + it('shows no send-blocked alert after a failed send attempt', async () => { store.dispatch(addAttachment(att({ id: 'bad-1', status: 'failed' }))); await render(); @@ -137,6 +218,26 @@ describe('MessageInput send with a failed attachment', () => { expect(container.querySelector('[role="alert"]')).toBeNull(); }); + it('fails a stalled upload and then flushes the queued send', async () => { + vi.useFakeTimers(); + await render(); + await attachFile('huge.pdf'); + expect(store.getState().upload.attachments[0].status).toBe('uploading'); + + await type('Is it done?'); + await pressEnter(); + // Pending: the send is queued, not dropped. + expect(onSubmit).not.toHaveBeenCalled(); + + await act(async () => { + vi.advanceTimersByTime(UPLOAD_STALL_TIMEOUT_MS); + }); + + expect(onSubmit).toHaveBeenCalledTimes(1); + expect(onSubmit).toHaveBeenCalledWith('Is it done?'); + expect(store.getState().upload.attachments).toEqual([]); + }); + it('keeps a queued question while another answer is streaming', async () => { store.dispatch(addAttachment(att({ status: 'processing', progress: 30 }))); await render(); diff --git a/frontend/src/components/MessageInput.tsx b/frontend/src/components/MessageInput.tsx index 05920510..1fd733df 100644 --- a/frontend/src/components/MessageInput.tsx +++ b/frontend/src/components/MessageInput.tsx @@ -51,6 +51,7 @@ import { ToolsTrigger, } from './message-input'; import { useArmedSend } from './message-input/armedSend'; +import { guardUploadStall } from './message-input/uploadStallGuard'; import { cannotReadAttachment } from './message-input/attachmentReadability'; import { handleAbort } from '../conversation/conversationSlice'; import { @@ -555,6 +556,7 @@ export default function MessageInput({ const files = supported; const apiHost = envVar('VITE_API_HOST'); + const uploadFailedMessage = t('conversation.attachments.uploadFailed'); if (files.length > 1) { const formData = new FormData(); @@ -830,20 +832,29 @@ export default function MessageInput({ } }; - xhr.onerror = () => { - console.error('Upload network error'); - Object.values(indexToUiId).forEach((id) => - dispatch( - updateAttachment({ - id, - updates: { status: 'failed' }, - }), - ), - ); - }; + // No response at all (status 0): a file the browser couldn't read, + // a dropped connection, or a stall the guard aborted. + xhr.onerror = + xhr.onabort = + xhr.ontimeout = + () => { + console.error('Upload network error'); + Object.values(indexToUiId).forEach((id) => + dispatch( + updateAttachment({ + id, + updates: { + status: 'failed', + errorMessage: uploadFailedMessage, + }, + }), + ), + ); + }; xhr.open('POST', `${apiHost}${endpoints.USER.STORE_ATTACHMENT}`); if (token) xhr.setRequestHeader('Authorization', `Bearer ${token}`); + guardUploadStall(xhr); xhr.send(formData); return; } @@ -951,17 +962,24 @@ export default function MessageInput({ } }; - xhr.onerror = () => { - dispatch( - updateAttachment({ - id: uniqueId, - updates: { status: 'failed' }, - }), - ); - }; + xhr.onerror = + xhr.onabort = + xhr.ontimeout = + () => { + dispatch( + updateAttachment({ + id: uniqueId, + updates: { + status: 'failed', + errorMessage: uploadFailedMessage, + }, + }), + ); + }; xhr.open('POST', `${apiHost}${endpoints.USER.STORE_ATTACHMENT}`); if (token) xhr.setRequestHeader('Authorization', `Bearer ${token}`); + guardUploadStall(xhr); xhr.send(formData); }); }, diff --git a/frontend/src/components/message-input/uploadStallGuard.test.ts b/frontend/src/components/message-input/uploadStallGuard.test.ts new file mode 100644 index 00000000..02262b5e --- /dev/null +++ b/frontend/src/components/message-input/uploadStallGuard.test.ts @@ -0,0 +1,54 @@ +import { guardUploadStall, UPLOAD_STALL_TIMEOUT_MS } from './uploadStallGuard'; + +class FakeXHR extends EventTarget { + upload = new EventTarget(); + abort = vi.fn(() => { + this.dispatchEvent(new Event('abort')); + this.dispatchEvent(new Event('loadend')); + }); +} + +const make = () => { + const xhr = new FakeXHR(); + guardUploadStall(xhr as unknown as XMLHttpRequest); + return xhr; +}; + +describe('guardUploadStall', () => { + beforeEach(() => vi.useFakeTimers()); + afterEach(() => vi.useRealTimers()); + + it('aborts an upload that never reports progress', () => { + const xhr = make(); + vi.advanceTimersByTime(UPLOAD_STALL_TIMEOUT_MS - 1); + expect(xhr.abort).not.toHaveBeenCalled(); + vi.advanceTimersByTime(1); + expect(xhr.abort).toHaveBeenCalledTimes(1); + }); + + it('lets a slow upload run while it keeps making progress', () => { + const xhr = make(); + for (let i = 0; i < 5; i += 1) { + vi.advanceTimersByTime(UPLOAD_STALL_TIMEOUT_MS - 1); + xhr.upload.dispatchEvent(new Event('progress')); + } + expect(xhr.abort).not.toHaveBeenCalled(); + }); + + it('gives the server a fresh window once the body is sent', () => { + const xhr = make(); + vi.advanceTimersByTime(UPLOAD_STALL_TIMEOUT_MS - 1); + xhr.upload.dispatchEvent(new Event('load')); + vi.advanceTimersByTime(UPLOAD_STALL_TIMEOUT_MS - 1); + expect(xhr.abort).not.toHaveBeenCalled(); + vi.advanceTimersByTime(1); + expect(xhr.abort).toHaveBeenCalledTimes(1); + }); + + it('stops watching once the request settles', () => { + const xhr = make(); + xhr.dispatchEvent(new Event('loadend')); + vi.advanceTimersByTime(UPLOAD_STALL_TIMEOUT_MS * 2); + expect(xhr.abort).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/components/message-input/uploadStallGuard.ts b/frontend/src/components/message-input/uploadStallGuard.ts new file mode 100644 index 00000000..bf79cad5 --- /dev/null +++ b/frontend/src/components/message-input/uploadStallGuard.ts @@ -0,0 +1,33 @@ +// How long an attachment upload may go without any sign of life. It is an +// idle window, not a total cap: a large file on a slow link keeps resetting +// it with progress events, so only a silent stall is cut off. +export const UPLOAD_STALL_TIMEOUT_MS = 120_000; + +/** + * Abort ``xhr`` once it makes no progress for ``idleMs``. + * + * ``xhr.timeout`` would cap the whole request, which a large file on a slow + * connection can legitimately exceed. This resets on every upload progress + * event, and again when the body finishes sending so the server gets its own + * window to answer. The abort fires the request's ``onabort`` handler, which + * is where the caller marks the attachment failed. + * + * Args: + * xhr: The request to watch. Call before ``send()``. + * idleMs: The longest allowed gap between signs of progress. + */ +export function guardUploadStall( + xhr: XMLHttpRequest, + idleMs: number = UPLOAD_STALL_TIMEOUT_MS, +): void { + let timer: ReturnType | undefined; + const restart = () => { + clearTimeout(timer); + timer = setTimeout(() => xhr.abort(), idleMs); + }; + xhr.upload.addEventListener('progress', restart); + xhr.upload.addEventListener('load', restart); + xhr.addEventListener('progress', restart); + xhr.addEventListener('loadend', () => clearTimeout(timer)); + restart(); +} diff --git a/frontend/src/locale/de.json b/frontend/src/locale/de.json index e788de44..7c35fe9b 100644 --- a/frontend/src/locale/de.json +++ b/frontend/src/locale/de.json @@ -1373,6 +1373,7 @@ "attachments": { "attach": "Anhängen", "remove": "Anhang entfernen", + "uploadFailed": "Upload fehlgeschlagen. Die Datei konnte nicht gelesen werden oder die Verbindung wurde unterbrochen.", "attached": "Anhang", "failed": "Fehlgeschlagen", "attachment": "Anhang" diff --git a/frontend/src/locale/en.json b/frontend/src/locale/en.json index fa295e00..986577a1 100644 --- a/frontend/src/locale/en.json +++ b/frontend/src/locale/en.json @@ -1379,6 +1379,7 @@ "attachments": { "attach": "Attach", "remove": "Remove attachment", + "uploadFailed": "Upload failed. The file couldn't be read or the connection dropped.", "waitingToSend_one": "Will send when {{count}} file finishes processing…", "waitingToSend_other": "Will send when {{count}} files finish processing…", "cancelQueuedSend": "Cancel", diff --git a/frontend/src/locale/es.json b/frontend/src/locale/es.json index 90b38b89..681e2e0d 100644 --- a/frontend/src/locale/es.json +++ b/frontend/src/locale/es.json @@ -1373,6 +1373,7 @@ "attachments": { "attach": "Adjuntar", "remove": "Eliminar adjunto", + "uploadFailed": "Error al subir. No se pudo leer el archivo o se perdió la conexión.", "attached": "Adjunto", "failed": "Error", "attachment": "Adjunto" diff --git a/frontend/src/locale/jp.json b/frontend/src/locale/jp.json index 41cf29f1..9e0fa1dd 100644 --- a/frontend/src/locale/jp.json +++ b/frontend/src/locale/jp.json @@ -1370,6 +1370,7 @@ "attachments": { "attach": "添付", "remove": "添付ファイルを削除", + "uploadFailed": "アップロードに失敗しました。ファイルを読み取れなかったか、接続が切断されました。", "attached": "添付ファイル", "failed": "失敗", "attachment": "添付ファイル" diff --git a/frontend/src/locale/ru.json b/frontend/src/locale/ru.json index 7a9d0352..c00face0 100644 --- a/frontend/src/locale/ru.json +++ b/frontend/src/locale/ru.json @@ -1417,6 +1417,7 @@ "attachments": { "attach": "Прикрепить", "remove": "Удалить вложение", + "uploadFailed": "Не удалось загрузить. Файл не удалось прочитать или соединение прервалось.", "attached": "Вложение", "failed": "Ошибка", "attachment": "Вложение" diff --git a/frontend/src/locale/zh-TW.json b/frontend/src/locale/zh-TW.json index ff1d2efc..3ca0570b 100644 --- a/frontend/src/locale/zh-TW.json +++ b/frontend/src/locale/zh-TW.json @@ -1370,6 +1370,7 @@ "attachments": { "attach": "附件", "remove": "刪除附件", + "uploadFailed": "上傳失敗。無法讀取檔案或連線已中斷。", "attached": "附件", "failed": "失敗", "attachment": "附件" diff --git a/frontend/src/locale/zh.json b/frontend/src/locale/zh.json index 36dc4e58..d41815c1 100644 --- a/frontend/src/locale/zh.json +++ b/frontend/src/locale/zh.json @@ -1370,6 +1370,7 @@ "attachments": { "attach": "附件", "remove": "删除附件", + "uploadFailed": "上传失败。无法读取文件或连接已断开。", "attached": "附件", "failed": "失败", "attachment": "附件" From dc5ebf5bb9d04800a22e935633e867685540f298 Mon Sep 17 00:00:00 2001 From: arc53-machine <232052973+arc53-machine@users.noreply.github.com> Date: Mon, 28 Sep 2026 10:06:25 +0100 Subject: [PATCH 4/7] e2e: a failed attachment no longer blocks the send Aborts /api/store_attachment, attaches a file, waits for the failed chip's reason, then sends. Checks that one /stream request goes out with no attachment ids, that the question is in the thread and the chip is gone, and that the stored message has no attachments. --- .../specs/tier-a/composer-resilience.spec.ts | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/tests/e2e/specs/tier-a/composer-resilience.spec.ts b/tests/e2e/specs/tier-a/composer-resilience.spec.ts index 5bdc9f39..2199ea69 100644 --- a/tests/e2e/specs/tier-a/composer-resilience.spec.ts +++ b/tests/e2e/specs/tier-a/composer-resilience.spec.ts @@ -14,6 +14,10 @@ * file arrives bound to the turn (`conversation_messages.attachments[]`) * instead of being silently dropped from the payload. * + * 3. Send with an attachment that failed. A failed file never resolves, + * so it must never hold the question: Send drops the failed chip and + * sends the text with whatever did upload. + * * 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. @@ -246,4 +250,74 @@ test.describe("tier-a · composer resilience", () => { await context.close(); } }); + + test("send with a failed attachment drops the file and sends the question", async ({ + browser, + }) => { + const { context, sub } = await newUserContext(browser); + try { + const page = await context.newPage(); + await openComposer(page); + + const streamPosts: string[] = []; + page.on("request", (req) => { + if (req.url().includes("/stream") && req.method() === "POST") { + streamPosts.push(req.postData() ?? ""); + } + }); + + // No response at all (status 0), the shape of the Android file-read + // failure seen in production. + await page.route("**/api/store_attachment", (route) => + route.abort("failed"), + ); + await page + .locator('input[type="file"]') + .first() + .setInputFiles(SMALL_FIXTURE_PATH); + + // The failed chip explains itself before Send. + await expect(page.getByText(/upload failed/i)).toBeVisible({ + timeout: 5_000, + }); + + const question = "does this still send? e2e-failed-attachment"; + const textarea = page.locator("#message-input"); + await textarea.fill(question); + const streamDone = page.waitForResponse( + (r) => r.url().includes("/stream") && r.request().method() === "POST", + { timeout: 30_000 }, + ); + await textarea.press("Enter"); + expect((await streamDone).status()).toBe(200); + + // Exactly one send, with no attachment ids. + expect(streamPosts).toHaveLength(1); + const payload = JSON.parse(streamPosts[0]) as { attachments?: string[] }; + expect(payload.attachments ?? []).toHaveLength(0); + + // The question is in the thread; the chip and its reason are gone. + await expect(page.getByText(question)).toBeVisible(); + await expect(page.getByText(/upload failed/i)).toBeHidden(); + await expect(page.getByText("notes.txt")).toBeHidden(); + await expect(textarea).toHaveValue(""); + + const { rows } = await pg.query<{ + prompt: string | null; + attachment_count: number | string | null; + }>( + `SELECT cm.prompt, + COALESCE(array_length(cm.attachments, 1), 0) AS attachment_count + 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(question); + expect(Number(rows[0].attachment_count)).toBe(0); + } finally { + await context.close(); + } + }); }); From b8a62cedf98aac3e5dc8915a33ed234984fea946 Mon Sep 17 00:00:00 2001 From: arc53-machine <232052973+arc53-machine@users.noreply.github.com> Date: Mon, 28 Sep 2026 10:27:12 +0100 Subject: [PATCH 5/7] Show a failed attachment's reason on hover, not as a red line The composer showed each failed file's reason as a red line of text under the chips. A failed file no longer blocks the send, since it is dropped then, so that line was louder than it needed to be. DESIGN.md also says status text of a sentence or more must not be a coloured paragraph. The chip's warning icon already marks the failure. The reason is now a Tooltip on the failed chip. It falls back to "Failed" when no reason is known. It is also kept as sr-only text so screen readers still read it. The native title moves from the chip to the truncating file name, where DESIGN.md keeps title=, and is left off failed chips so the tooltip doesn't double up. --- .../message-input/AttachmentChipList.test.tsx | 102 ++++++++++++++++++ .../message-input/AttachmentChipList.tsx | 49 ++++----- .../specs/tier-a/composer-resilience.spec.ts | 13 ++- 3 files changed, 133 insertions(+), 31 deletions(-) create mode 100644 frontend/src/components/message-input/AttachmentChipList.test.tsx diff --git a/frontend/src/components/message-input/AttachmentChipList.test.tsx b/frontend/src/components/message-input/AttachmentChipList.test.tsx new file mode 100644 index 00000000..a2f7b799 --- /dev/null +++ b/frontend/src/components/message-input/AttachmentChipList.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 type { Attachment } from '../../upload/uploadSlice'; +import AttachmentChipList from './AttachmentChipList'; + +Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); + +const att = (over: Partial = {}): Attachment => ({ + id: 'a1', + fileName: 'scan.pdf', + progress: 0, + status: 'failed', + taskId: '', + errorMessage: 'Upload failed. The file could not be read.', + ...over, +}); + +describe('AttachmentChipList failed chip', () => { + let container: HTMLDivElement; + let root: Root; + + beforeEach(() => { + vi.useFakeTimers(); + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + }); + + afterEach(async () => { + await act(async () => root.unmount()); + container.remove(); + vi.useRealTimers(); + }); + + const render = async (attachments: Attachment[]) => { + await act(async () => { + root.render( + {}} + onDragStart={() => {}} + onDragOver={() => {}} + onDropOn={() => {}} + />, + ); + }); + }; + + const tooltip = () => + document.body.querySelector('[data-slot="tooltip-content"]'); + + it('shows no inline failure line under the chips', async () => { + await render([att()]); + + expect(container.querySelector('[role="alert"]')).toBeNull(); + expect(container.querySelector('.text-destructive')).toBeNull(); + }); + + it('shows the reason in a tooltip when the chip is hovered', async () => { + await render([att()]); + expect(tooltip()).toBeNull(); + + // The remove X is an IconButton with its own tooltip; the chip is the div. + const chip = container.querySelector('div[data-slot="tooltip-trigger"]')!; + await act(async () => { + chip.dispatchEvent( + new PointerEvent('pointermove', { + bubbles: true, + pointerType: 'mouse', + }), + ); + vi.advanceTimersByTime(500); + }); + + expect(tooltip()?.textContent).toContain( + 'Upload failed. The file could not be read.', + ); + }); + + it('keeps the reason available to screen readers', async () => { + await render([att()]); + + const srOnly = container.querySelector('.sr-only'); + expect(srOnly?.textContent).toBe( + 'Upload failed. The file could not be read.', + ); + }); + + it('adds no tooltip to a chip that did not fail', async () => { + await render([att({ status: 'completed', errorMessage: undefined })]); + + expect( + container.querySelector('div[data-slot="tooltip-trigger"]'), + ).toBeNull(); + }); +}); diff --git a/frontend/src/components/message-input/AttachmentChipList.tsx b/frontend/src/components/message-input/AttachmentChipList.tsx index 7d31dfc9..bda4e01a 100644 --- a/frontend/src/components/message-input/AttachmentChipList.tsx +++ b/frontend/src/components/message-input/AttachmentChipList.tsx @@ -3,6 +3,7 @@ import { useTranslation } from 'react-i18next'; import type { Attachment } from '../../upload/uploadSlice'; import { IconButton } from '../ui/icon-button'; +import { Tooltip, TooltipContent, TooltipTrigger } from '../ui/tooltip'; import { cn } from '@/lib/utils'; type AttachmentChipListProps = { @@ -30,12 +31,6 @@ export default function AttachmentChipList({ }: AttachmentChipListProps) { const { t } = useTranslation(); - // A tooltip is the one place a touch user can never look, and this list is - // where a phone picker's unsupported file lands. Show the reason inline, - // as soon as it is known, rather than only once a send is attempted. - const failures = attachments.filter( - (attachment) => attachment.status === 'failed' && attachment.errorMessage, - ); // Not a failure: the file is kept and still sends. It only warns that the // model picked right now would receive nothing it can read. const unreadable = modelName @@ -46,7 +41,14 @@ export default function AttachmentChipList({ <>
{attachments.map((attachment) => { - return ( + const failed = attachment.status === 'failed'; + // A failed file never blocks the send (it is dropped then), so its + // reason stays out of the way: the chip's warning icon marks it, and + // the reason is a hover tooltip plus screen-reader text. + const failureReason = failed + ? (attachment.errorMessage ?? t('conversation.attachments.failed')) + : null; + const chip = (
{attachment.status === 'completed' && ( @@ -115,9 +112,15 @@ export default function AttachmentChipList({ )}
- + {attachment.fileName} + {failureReason && ( + {failureReason} + )}
); + if (!failureReason) return chip; + return ( + + {chip} + {failureReason} + + ); })}
- {failures.length > 0 && ( -
- {failures.map((attachment) => ( - - {attachment.fileName}: {attachment.errorMessage} - - ))} -
- )} - {unreadable.length > 0 && (
{ .first() .setInputFiles(SMALL_FIXTURE_PATH); - // The failed chip explains itself before Send. - await expect(page.getByText(/upload failed/i)).toBeVisible({ - timeout: 5_000, - }); + // The failed chip is marked by its warning icon; the reason is a + // hover tooltip, not a line of text in the composer. + const failedIcon = page.getByLabel("Failed", { exact: true }); + await expect(failedIcon).toBeVisible({ timeout: 5_000 }); + await failedIcon.hover(); + await expect(page.getByRole("tooltip")).toContainText(/upload failed/i); const question = "does this still send? e2e-failed-attachment"; const textarea = page.locator("#message-input"); @@ -298,7 +300,8 @@ test.describe("tier-a · composer resilience", () => { // The question is in the thread; the chip and its reason are gone. await expect(page.getByText(question)).toBeVisible(); - await expect(page.getByText(/upload failed/i)).toBeHidden(); + await expect(failedIcon).toHaveCount(0); + await expect(page.getByText(/upload failed/i)).toHaveCount(0); await expect(page.getByText("notes.txt")).toBeHidden(); await expect(textarea).toHaveValue(""); From 7e990d43848f5d190ce3c312fe2a3ab698c03bc1 Mon Sep 17 00:00:00 2001 From: arc53-machine <232052973+arc53-machine@users.noreply.github.com> Date: Mon, 28 Sep 2026 10:37:09 +0100 Subject: [PATCH 6/7] Name the file in a failed chip's tooltip and announce failures Follow-ups to moving the failure reason into a tooltip: - The tooltip now reads ": ". The name is truncated on the chip, and two long names can look the same. - A polite role="status" region, always mounted and screen-reader only, lists the failed files and their reasons. The red line it replaces was role="alert", so failures were announced; a hover tooltip is not. - The reason now describes the remove button (aria-describedby on a hidden span), the chip's only focusable control. Before, it was sr-only text on a chip that can't take focus. --- .../message-input/AttachmentChipList.test.tsx | 27 +++++++++++--- .../message-input/AttachmentChipList.tsx | 35 +++++++++++++++---- 2 files changed, 51 insertions(+), 11 deletions(-) diff --git a/frontend/src/components/message-input/AttachmentChipList.test.tsx b/frontend/src/components/message-input/AttachmentChipList.test.tsx index a2f7b799..b117f6bc 100644 --- a/frontend/src/components/message-input/AttachmentChipList.test.tsx +++ b/frontend/src/components/message-input/AttachmentChipList.test.tsx @@ -78,16 +78,35 @@ describe('AttachmentChipList failed chip', () => { vi.advanceTimersByTime(500); }); + // The full name, since two long names can truncate to the same text. expect(tooltip()?.textContent).toContain( - 'Upload failed. The file could not be read.', + 'scan.pdf: Upload failed. The file could not be read.', ); }); - it('keeps the reason available to screen readers', async () => { + it('announces failures in a persistent polite live region', async () => { + await render([att({ status: 'uploading', errorMessage: undefined })]); + const region = container.querySelector('[role="status"]'); + expect(region?.textContent).toBe(''); + await render([att()]); - const srOnly = container.querySelector('.sr-only'); - expect(srOnly?.textContent).toBe( + // Same node, updated in place, so screen readers announce the change. + expect(container.querySelector('[role="status"]')).toBe(region); + expect(region?.textContent).toBe( + 'scan.pdf: Upload failed. The file could not be read.', + ); + }); + + it('describes the focusable remove button with the reason', async () => { + await render([att()]); + + const remove = container.querySelector( + 'button[aria-label="conversation.attachments.remove"]', + )!; + const describedBy = remove.getAttribute('aria-describedby'); + expect(describedBy).toBeTruthy(); + expect(document.getElementById(describedBy!)?.textContent).toBe( 'Upload failed. The file could not be read.', ); }); diff --git a/frontend/src/components/message-input/AttachmentChipList.tsx b/frontend/src/components/message-input/AttachmentChipList.tsx index bda4e01a..f6d6cd48 100644 --- a/frontend/src/components/message-input/AttachmentChipList.tsx +++ b/frontend/src/components/message-input/AttachmentChipList.tsx @@ -1,4 +1,5 @@ import { Paperclip, TriangleAlert, X } from 'lucide-react'; +import { useId } from 'react'; import { useTranslation } from 'react-i18next'; import type { Attachment } from '../../upload/uploadSlice'; @@ -30,6 +31,9 @@ export default function AttachmentChipList({ modelName, }: AttachmentChipListProps) { const { t } = useTranslation(); + const reasonIdPrefix = useId(); + const failureReasonOf = (attachment: Attachment) => + attachment.errorMessage ?? t('conversation.attachments.failed'); // Not a failure: the file is kept and still sends. It only warns that the // model picked right now would receive nothing it can read. @@ -43,11 +47,11 @@ export default function AttachmentChipList({ {attachments.map((attachment) => { const failed = attachment.status === 'failed'; // A failed file never blocks the send (it is dropped then), so its - // reason stays out of the way: the chip's warning icon marks it, and - // the reason is a hover tooltip plus screen-reader text. - const failureReason = failed - ? (attachment.errorMessage ?? t('conversation.attachments.failed')) - : null; + // reason stays out of the way: the chip's warning icon marks it, the + // reason is a hover tooltip, and it describes the remove button (the + // chip's one focusable control) for keyboard and screen readers. + const failureReason = failed ? failureReasonOf(attachment) : null; + const reasonId = `${reasonIdPrefix}-${attachment.id}`; const chip = (
{failureReason && ( - {failureReason} + )} { onRemove(attachment.id); }} @@ -140,12 +147,26 @@ export default function AttachmentChipList({ return ( {chip} - {failureReason} + + {attachment.fileName}: {failureReason} + ); })}
+ {/* Always mounted, so a chip turning failed is announced: the reason + itself is only on hover, which a screen reader never sees. */} +
+ {attachments + .filter((attachment) => attachment.status === 'failed') + .map( + (attachment) => + `${attachment.fileName}: ${failureReasonOf(attachment)}`, + ) + .join(' ')} +
+ {unreadable.length > 0 && (
Date: Mon, 28 Sep 2026 10:47:30 +0100 Subject: [PATCH 7/7] Name the file in the remove button's failure description The remove button's label is "Remove attachment" and doesn't say which file. Its failure description now reads ": ", the same text as the tooltip. --- .../src/components/message-input/AttachmentChipList.test.tsx | 3 ++- frontend/src/components/message-input/AttachmentChipList.tsx | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/frontend/src/components/message-input/AttachmentChipList.test.tsx b/frontend/src/components/message-input/AttachmentChipList.test.tsx index b117f6bc..df8a5a1d 100644 --- a/frontend/src/components/message-input/AttachmentChipList.test.tsx +++ b/frontend/src/components/message-input/AttachmentChipList.test.tsx @@ -106,8 +106,9 @@ describe('AttachmentChipList failed chip', () => { )!; const describedBy = remove.getAttribute('aria-describedby'); expect(describedBy).toBeTruthy(); + // Named, since the button's own label doesn't say which file. expect(document.getElementById(describedBy!)?.textContent).toBe( - 'Upload failed. The file could not be read.', + 'scan.pdf: Upload failed. The file could not be read.', ); }); diff --git a/frontend/src/components/message-input/AttachmentChipList.tsx b/frontend/src/components/message-input/AttachmentChipList.tsx index f6d6cd48..6376e7fd 100644 --- a/frontend/src/components/message-input/AttachmentChipList.tsx +++ b/frontend/src/components/message-input/AttachmentChipList.tsx @@ -124,7 +124,7 @@ export default function AttachmentChipList({ {failureReason && ( )}