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.
This commit is contained in:
arc53-machine committed 2026-09-28 10:05:19 +01:00
1 parent d37e0bd1ad
commit a9c0082e9a
7 files changed
+203 -44

No files matched your search

@@ -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<typeof makeStore>;
const att = (over: Partial<Attachment> = {}): 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<typeof vi.fn<(text: string) => 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(
<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 }),
);
});
};
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();
});
});
+20 -16
View File
@@ -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({
</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}
@@ -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 () => {
@@ -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;
+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
@@ -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",