Merge pull request #2839 from arc53-machine/failed-attachment-never-blocks-send

A failed attachment never blocks Send: drop it and send the question
This commit is contained in:
Alex authored and GitHub committed 2026-09-28 12:21:30 +01:00
commit aa0ce42fd7
18 files changed
+740 -93

No files matched your search

@@ -0,0 +1,265 @@
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';
import { UPLOAD_STALL_TIMEOUT_MS } from './message-input/uploadStallGuard';
Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true });
const makeStore = () =>
configureStore({
reducer: {
preference: prefSlice.reducer,
upload: uploadReducer,
notifications: notificationsReducer,
},
});
type TestStore = ReturnType<typeof makeStore>;
const att = (over: Partial<Attachment> = {}): Attachment => ({
id: 'ok-1',
fileName: 'notes.pdf',
progress: 100,
status: 'completed',
taskId: 't1',
...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<typeof vi.fn<(text: string) => 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);
store = makeStore();
onSubmit = vi.fn<(text: string) => void>();
});
afterEach(async () => {
await act(async () => root.unmount());
container.remove();
globalThis.XMLHttpRequest = realXHR;
vi.useRealTimers();
});
const render = async (loading = false) => {
await act(async () => {
root.render(
<Provider store={store}>
<MessageInput
onSubmit={onSubmit}
loading={loading}
showSourceButton={false}
showToolButton={false}
autoFocus={false}
/>
</Provider>,
);
});
};
const textarea = () =>
container.querySelector<HTMLTextAreaElement>('#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 }),
);
});
};
const attachFile = async (name: string) => {
const input = container.querySelector<HTMLInputElement>(
'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(
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('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();
await type('hello');
await pressEnter();
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();
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');
});
});
+67 -38
View File
@@ -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);
});
},
@@ -1578,17 +1596,30 @@ 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);
const canSubmit =
hasSubmittableContent &&
Boolean(value.trim()) ||
(allowSendWithoutText && attachments.some((a) => a.status !== 'failed'));
const composerIdle =
!loading &&
recordingState !== 'recording' &&
recordingState !== 'transcribing';
const canSubmit = hasSubmittableContent && composerIdle;
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) {
@@ -1607,7 +1638,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
@@ -1621,9 +1659,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 +1761,6 @@ export default function MessageInput({
</Button>
</div>
)}
{sendArmed && sendReadiness.state === 'blocked' && (
<div
className="text-destructive px-2 pb-1 text-xs sm:px-3"
role="alert"
>
{t('conversation.attachments.sendBlockedByFailed', {
names: sendReadiness.failedNames.join(', '),
})}
</div>
)}
{voiceError && (
<div className="text-destructive px-2 pb-1 text-xs sm:px-3">
{voiceError}
@@ -0,0 +1,122 @@
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> = {}): 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(
<AttachmentChipList
attachments={attachments}
draggingId={null}
onRemove={() => {}}
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);
});
// The full name, since two long names can truncate to the same text.
expect(tooltip()?.textContent).toContain(
'scan.pdf: Upload failed. The file could not be read.',
);
});
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()]);
// 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();
// Named, since the button's own label doesn't say which file.
expect(document.getElementById(describedBy!)?.textContent).toBe(
'scan.pdf: 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();
});
});
@@ -1,8 +1,10 @@
import { Paperclip, TriangleAlert, X } from 'lucide-react';
import { useId } from 'react';
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 = {
@@ -29,13 +31,10 @@ export default function AttachmentChipList({
modelName,
}: AttachmentChipListProps) {
const { t } = useTranslation();
const reasonIdPrefix = useId();
const failureReasonOf = (attachment: Attachment) =>
attachment.errorMessage ?? t('conversation.attachments.failed');
// 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 +45,14 @@ export default function AttachmentChipList({
<>
<div className="flex flex-wrap gap-1.5 px-2 py-2 sm:gap-2 sm:px-3">
{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, 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 = (
<div
key={attachment.id}
draggable={true}
@@ -62,11 +68,6 @@ export default function AttachmentChipList({
: 'opacity-100',
draggingId === attachment.id && 'ring-primary/30 ring-2',
)}
title={
attachment.status === 'failed' && attachment.errorMessage
? `${attachment.fileName}: ${attachment.errorMessage}`
: attachment.fileName
}
>
<div className="bg-primary mr-2 flex size-8 items-center justify-center rounded-md p-1">
{attachment.status === 'completed' && (
@@ -115,9 +116,17 @@ export default function AttachmentChipList({
)}
</div>
<span className="max-w-[120px] truncate font-medium sm:max-w-[150px]">
<span
className="max-w-[120px] truncate font-medium sm:max-w-[150px]"
title={failed ? undefined : attachment.fileName}
>
{attachment.fileName}
</span>
{failureReason && (
<span id={reasonId} hidden>
{attachment.fileName}: {failureReason}
</span>
)}
<IconButton
label={t('conversation.attachments.remove')}
@@ -125,6 +134,7 @@ export default function AttachmentChipList({
size="icon-xs"
shape="pill"
className="ml-1.5"
aria-describedby={failureReason ? reasonId : undefined}
onClick={() => {
onRemove(attachment.id);
}}
@@ -133,21 +143,29 @@ export default function AttachmentChipList({
</IconButton>
</div>
);
if (!failureReason) return chip;
return (
<Tooltip key={attachment.id}>
<TooltipTrigger asChild>{chip}</TooltipTrigger>
<TooltipContent>
{attachment.fileName}: {failureReason}
</TooltipContent>
</Tooltip>
);
})}
</div>
{failures.length > 0 && (
<div
className="text-destructive flex flex-col gap-0.5 px-2 pb-1 text-xs sm:px-3"
role="alert"
>
{failures.map((attachment) => (
<span key={attachment.id}>
{attachment.fileName}: {attachment.errorMessage}
</span>
))}
</div>
)}
{/* Always mounted, so a chip turning failed is announced: the reason
itself is only on hover, which a screen reader never sees. */}
<div className="sr-only" role="status" aria-live="polite">
{attachments
.filter((attachment) => attachment.status === 'failed')
.map(
(attachment) =>
`${attachment.fileName}: ${failureReasonOf(attachment)}`,
)
.join(' ')}
</div>
{unreadable.length > 0 && (
<div
@@ -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 });
});
});
@@ -60,13 +66,15 @@ type HookApi = ReturnType<typeof useArmedSend>;
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;
}
@@ -90,10 +98,15 @@ describe('useArmedSend', () => {
container.remove();
});
const render = async (attachments: Attachment[]) => {
const render = async (attachments: Attachment[], canFlush?: boolean) => {
await act(async () => {
root.render(
<Host attachments={attachments} onFlush={onFlush} api={api} />,
<Host
attachments={attachments}
onFlush={onFlush}
canFlush={canFlush}
api={api}
/>,
);
});
};
@@ -120,19 +133,30 @@ describe('useArmedSend', () => {
expect(onFlush).toHaveBeenCalledTimes(1);
});
it('holds the flush while a file is failed and resumes when it is removed', async () => {
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).toHaveBeenCalledTimes(1);
expect(api.current!.armed).toBe(false);
await render([att({ status: 'failed', fileName: 'broken.pdf' })]);
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());
await render([att({ status: 'failed', fileName: 'broken.pdf' })]);
// 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);
expect(api.current!.readiness).toEqual({
state: 'blocked',
failedNames: ['broken.pdf'],
});
await render([]);
await render([att()], true);
expect(onFlush).toHaveBeenCalledTimes(1);
expect(api.current!.armed).toBe(false);
});
@@ -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;
@@ -24,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,
@@ -42,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), []);
@@ -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();
});
});
@@ -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<typeof setTimeout> | 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();
}
+6 -4
View File
@@ -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],
@@ -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', () => {
+1
View File
@@ -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"
+1 -1
View File
@@ -1379,10 +1379,10 @@
"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",
"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",
+1
View File
@@ -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"
+1
View File
@@ -1370,6 +1370,7 @@
"attachments": {
"attach": "添付",
"remove": "添付ファイルを削除",
"uploadFailed": "アップロードに失敗しました。ファイルを読み取れなかったか、接続が切断されました。",
"attached": "添付ファイル",
"failed": "失敗",
"attachment": "添付ファイル"
+1
View File
@@ -1417,6 +1417,7 @@
"attachments": {
"attach": "Прикрепить",
"remove": "Удалить вложение",
"uploadFailed": "Не удалось загрузить. Файл не удалось прочитать или соединение прервалось.",
"attached": "Вложение",
"failed": "Ошибка",
"attachment": "Вложение"
+1
View File
@@ -1370,6 +1370,7 @@
"attachments": {
"attach": "附件",
"remove": "刪除附件",
"uploadFailed": "上傳失敗。無法讀取檔案或連線已中斷。",
"attached": "附件",
"failed": "失敗",
"attachment": "附件"
+1
View File
@@ -1370,6 +1370,7 @@
"attachments": {
"attach": "附件",
"remove": "删除附件",
"uploadFailed": "上传失败。无法读取文件或连接已断开。",
"attached": "附件",
"failed": "失败",
"attachment": "附件"
@@ -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,77 @@ 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 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");
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(failedIcon).toHaveCount(0);
await expect(page.getByText(/upload failed/i)).toHaveCount(0);
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();
}
});
});