fix: isolate attachment submission state

This commit is contained in:
2026-08-24 01:50:04 +08:00
parent bec93d0918
commit 1ca0249e69
8 changed files with 320 additions and 21 deletions

View File

@@ -151,6 +151,14 @@
behind a buffered continuous tail, Renderer requests one fresh target
Snapshot; a repeated identical stale response becomes a retryable error
instead of remaining in an infinite recovering state.
- Closed the incremental planner review findings after `bec93d0`: attachment
editing is locked for the originating Conversation until upload and HTTP 202
acceptance settle, so removal or a next-message paste cannot mutate the
captured image set. Attachment preparation failures now remain local to the
Composer, restore the draft, permit direct retry without runtime recovery,
and retain already uploaded variants for reuse. Attachment route failures use
only registered `CODING_ATTACHMENT_INVALID`, `CODING_ATTACHMENT_NOT_FOUND`,
and `CODING_STORAGE_WRITE_FAILED` codes while preserving 413/404 status.
- Synchronized `README.md` with the current Pi core Chat, first-Conversation,
batch and attachment behavior. PI-130 feature controls and PI-140 legacy
removal remain outside this task.
@@ -161,6 +169,10 @@
**Standards Needs Fix / Spec Needs Fix**. PI-120 remained the unique Ready
Frontier; the findings above were treated as blocking rather than advancing
PI-130.
- The correction candidate `bec93d0` received incremental planner review result
**Standards PASS / Spec Needs Fix**. Its three remaining attachment-state and
typed-error findings were implemented and kept PI-130 locked pending another
planner review.
- Post-review focused suites passed: attachment Host routes, stale recovery,
shared ProjectService/Registry mutation queue, slow Agent switch, cross-
Conversation submission isolation, attachment count/concurrency and pressure
@@ -171,7 +183,11 @@
20 React commits, 131,148 wire bytes, and measured Main-to-React p95
`35.995 ms` against the `<=50 ms` budget. It includes mixed message blocks,
4 KiB tool output and more than 100 KiB of cumulative thinking output.
- `pnpm test`: passed, 218 files / 2316 passed / 2 skipped.
- Latest attachment-state focused run passed: 3 files / 39 tests, including
delayed upload/removal, delayed HTTP 202/next-image isolation, local
preparation failure/direct retry, uploaded-variant reuse, and registered
Host error codes.
- `pnpm test`: passed, 218 files / 2321 passed / 2 skipped.
- `pnpm run typecheck`: passed.
- `pnpm run lint:check`: passed with 0 errors and six unchanged warnings in
`ExecutionGraphCard`, `Home`, and `Makelore`.

View File

@@ -66,28 +66,32 @@ async function readBoundedBody(req: IncomingMessage): Promise<Uint8Array> {
return Buffer.concat(chunks);
}
function sendAttachmentError(res: ServerResponse, error: unknown): void {
const status = typeof (error as { status?: unknown })?.status === 'number'
function sendAttachmentError(
res: ServerResponse,
error: unknown,
operation: 'upload' | 'content',
): void {
const explicitStatus = typeof (error as { status?: unknown })?.status === 'number'
? (error as { status: number }).status
: (error as NodeJS.ErrnoException)?.code === 'ENOENT'
? 404
: 500;
: null;
const status = explicitStatus
?? (operation === 'content' || (error as NodeJS.ErrnoException)?.code === 'ENOENT' ? 404 : 500);
const message = status === 413
? '图片不能超过 16 MB。'
: status === 404
? '图片附件不存在。'
: status === 400
? '图片附件无效。'
: '图片附件暂时无法读取。';
: operation === 'upload'
? '本地数据写入失败,请检查存储后重试。'
: '图片附件暂时无法读取。';
sendJson(res, status, {
success: false,
code: status === 413
? 'CODING_ATTACHMENT_TOO_LARGE'
code: status === 400 || status === 413
? 'CODING_ATTACHMENT_INVALID'
: status === 404
? 'CODING_ATTACHMENT_NOT_FOUND'
: status === 400
? 'CODING_ATTACHMENT_INVALID'
: 'CODING_ATTACHMENT_STORAGE_FAILED',
: 'CODING_STORAGE_WRITE_FAILED',
error: message,
});
}
@@ -147,7 +151,7 @@ export async function handleCodingAttachmentRoutes(
res.end(record.data);
return true;
} catch (error) {
sendAttachmentError(res, error);
sendAttachmentError(res, error, upload ? 'upload' : 'content');
return true;
}
}

View File

@@ -301,7 +301,8 @@ export function CodingChatPanel({
]);
const handleAddFiles = useCallback((files: File[]) => {
if (!draftKey) return;
if (!draftKey
|| (targetConversationId && submissionFlightsRef.current.has(targetConversationId))) return;
const accepted: LocalComposerAttachment[] = [];
const remaining = Math.max(0, CODING_ATTACHMENT_MAX_COUNT - localAttachments.length);
let validationError = files.length > remaining
@@ -335,10 +336,11 @@ export function CodingChatPanel({
...current,
[draftKey]: [...(current[draftKey] ?? []), ...accepted],
}));
}, [draftKey, localAttachments.length]);
}, [draftKey, localAttachments.length, targetConversationId]);
const handleRemoveAttachment = useCallback((id: string) => {
if (!draftKey) return;
if (!draftKey
|| (targetConversationId && submissionFlightsRef.current.has(targetConversationId))) return;
setAttachmentsByDraftKey((current) => {
const attachment = current[draftKey]?.find((item) => item.id === id);
if (attachment) URL.revokeObjectURL(attachment.previewUrl);
@@ -349,7 +351,12 @@ export function CodingChatPanel({
});
uploadedAttachmentsRef.current.delete(id);
uploadFlightsRef.current.delete(id);
}, [draftKey]);
setSubmissionErrors((current) => {
const next = { ...current };
delete next[draftKey];
return next;
});
}, [draftKey, targetConversationId]);
const prepareAttachments = useCallback(async (
attachments: LocalComposerAttachment[],

View File

@@ -91,6 +91,7 @@ export function CodingComposer({
type="file"
accept="image/png,image/jpeg,image/webp,image/gif"
multiple
disabled={!editable || submitting}
className="sr-only"
data-testid="coding-file-attachment-input"
onChange={(event) => {
@@ -110,8 +111,9 @@ export function CodingComposer({
/>
<button
type="button"
className="absolute right-1 top-1 flex h-10 w-10 translate-x-2 -translate-y-2 items-end justify-start rounded-full bg-foreground/85 p-1.5 text-background transition-[background-color,scale] duration-150 ease-out hover:bg-foreground active:scale-[0.96]"
className="absolute right-1 top-1 flex h-10 w-10 translate-x-2 -translate-y-2 items-end justify-start rounded-full bg-foreground/85 p-1.5 text-background transition-[background-color,scale] duration-150 ease-out hover:bg-foreground active:scale-[0.96] disabled:cursor-not-allowed disabled:opacity-60"
aria-label={`移除图片 ${attachment.name}`}
disabled={submitting}
onClick={() => onRemoveAttachment(attachment.id)}
>
<X className="h-3.5 w-3.5" aria-hidden="true" />
@@ -133,6 +135,7 @@ export function CodingComposer({
if (canSend) onSubmit();
}}
onPaste={(event) => {
if (submitting) return;
const files = Array.from(event.clipboardData.files).filter((file) => file.type.startsWith('image/'));
if (files.length > 0) onAddFiles(files);
}}

View File

@@ -702,10 +702,12 @@ export function createCodingConversationStore(
};
});
let attachmentsPrepared = !input.prepareAttachments;
try {
if (input.prepareAttachments) {
submittedDraft.attachments = (await input.prepareAttachments())
.map((attachment) => ({ ...attachment }));
attachmentsPrepared = true;
set((state) => {
const currentEntry = state.entriesByConversationId[input.conversationId] ?? emptyEntry();
const requests = state.requestsByConversationId[input.conversationId] ?? {};
@@ -765,6 +767,7 @@ export function createCodingConversationStore(
} catch (error) {
const failure = errorDetails(error);
const uncertain = failure.code === 'CODING_REQUEST_UNCERTAIN';
const preSubmitFailure = !attachmentsPrepared;
set((state) => {
const currentEntry = state.entriesByConversationId[input.conversationId] ?? emptyEntry();
const requests = state.requestsByConversationId[input.conversationId] ?? {};
@@ -776,7 +779,11 @@ export function createCodingConversationStore(
const reducer = uncertain
? currentEntry.reducer
: withRejectedNode(currentEntry.reducer, nodeId);
const nextEntry = { ...currentEntry, reducer, error: failure.message };
const nextEntry = {
...currentEntry,
reducer,
error: preSubmitFailure ? currentEntry.error : failure.message,
};
return {
entriesByConversationId: {
...state.entriesByConversationId,

View File

@@ -3,7 +3,7 @@ import { once } from 'node:events';
import { mkdtemp, rm } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import path from 'node:path';
import { afterEach, describe, expect, it } from 'vitest';
import { afterEach, describe, expect, it, vi } from 'vitest';
import { handleCodingAttachmentRoutes } from '../../electron/api/routes/coding-attachments';
import { getHostApiToken, startHostApiServer } from '../../electron/api/server';
import { CodingAttachmentStore } from '../../electron/coding-projects/attachment-store';
@@ -44,6 +44,37 @@ async function startAttachmentServer(maxBytes = 16 * 1024 * 1024) {
};
}
async function startAttachmentFailureServer() {
const attachments = {
put: vi.fn(async () => {
throw new Error('write failed');
}),
read: vi.fn(async () => {
throw new Error('read failed');
}),
};
const context = { codingProducts: { attachments } } as unknown as HostApiContext;
const server = createServer((request, response) => {
const url = new URL(request.url ?? '/', 'http://127.0.0.1');
void handleCodingAttachmentRoutes(request, response, url, context).then((handled) => {
if (!handled) {
response.statusCode = 404;
response.end();
}
});
});
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve));
const address = server.address();
if (!address || typeof address === 'string') throw new Error('Attachment test server failed');
return {
baseUrl: `http://127.0.0.1:${address.port}`,
close: async () => await new Promise<void>((resolve, reject) => {
server.closeAllConnections();
server.close((error) => error ? reject(error) : resolve());
}),
};
}
async function startAuthenticatedHostServer() {
const root = await mkdtemp(path.join(tmpdir(), 'makelore-coding-host-attachments-'));
roots.push(root);
@@ -222,7 +253,33 @@ describe('coding attachment routes', () => {
);
expect(response.status).toBe(413);
expect(response.body).toMatchObject({
code: 'CODING_ATTACHMENT_TOO_LARGE',
code: 'CODING_ATTACHMENT_INVALID',
});
} finally {
await server.close();
}
});
it('uses registered errors for unreadable content and upload storage failures', async () => {
const server = await startAttachmentFailureServer();
try {
const content = await fetch(
`${server.baseUrl}/api/coding/attachments/attachment-1/content`,
);
expect(content.status).toBe(404);
await expect(content.json()).resolves.toMatchObject({
code: 'CODING_ATTACHMENT_NOT_FOUND',
});
const upload = await fetch(`${server.baseUrl}/api/coding/attachments`, {
method: 'POST',
headers: { 'Content-Type': 'image/png' },
body: new Uint8Array([137, 80, 78, 71, 13, 10, 26, 10]),
});
expect(upload.status).toBe(500);
await expect(upload.json()).resolves.toMatchObject({
code: 'CODING_STORAGE_WRITE_FAILED',
error: '本地数据写入失败,请检查存储后重试。',
});
} finally {
await server.close();

View File

@@ -365,6 +365,162 @@ describe('CodingChatPanel first Conversation', () => {
expect(maxActiveUploads).toBeLessThanOrEqual(4);
});
it('locks attachment removal while an upload is still pending', async () => {
const upload = deferred<{ attachmentId: string; mime: string; byteLength: number }>();
projectApi.list.mockResolvedValue({ projects: [project], activeProjectId: project.id });
projectApi.config.mockResolvedValue({ project, config });
projectApi.conversations.mockResolvedValue([conversation]);
conversationApi.events.mockResolvedValue(new FakeEventSource() as unknown as EventSource);
conversationApi.recover.mockResolvedValue(undefined);
conversationApi.submit.mockImplementation(async (input: {
conversationId: string;
clientRequestId: string;
mode: 'prompt';
}) => ({
accepted: true,
conversationId: input.conversationId,
clientRequestId: input.clientRequestId,
runId: 'run-1',
mode: input.mode,
}));
attachmentApi.upload.mockReturnValue(upload.promise);
const { CodingChatPanel } = await import('@/pages/Chat/CodingChatPanel');
const { createLocalConversationSnapshot } = await import('@/pages/Chat/coding-chat-snapshot');
const { codingConversationStore } = await import('@/stores/coding-conversations');
conversationApi.snapshot.mockResolvedValue(createLocalConversationSnapshot(project.id, conversation));
render(<CodingChatPanel />);
await screen.findByRole('textbox');
await waitFor(() => expect(codingConversationStore.getState().selectedConversationId)
.toBe(conversation.id));
const file = new File(['image'], 'held.png', { type: 'image/png' });
fireEvent.change(screen.getByTestId('coding-file-attachment-input'), {
target: { files: [file] },
});
fireEvent.click(await screen.findByRole('button', { name: '发送' }));
await waitFor(() => expect(attachmentApi.upload).toHaveBeenCalledOnce());
const remove = screen.getByRole('button', { name: '移除图片 held.png' });
expect(remove).toBeDisabled();
fireEvent.click(remove);
expect(screen.getByAltText('held.png')).toBeInTheDocument();
upload.resolve({ attachmentId: 'attachment-held', mime: 'image/png', byteLength: 5 });
await waitFor(() => expect(conversationApi.submit).toHaveBeenCalledOnce());
await waitFor(() => expect(screen.queryByAltText('held.png')).not.toBeInTheDocument());
});
it('keeps next-message images out of a submission while its 202 response is pending', async () => {
const acceptanceFlight = deferred<{
accepted: true;
conversationId: string;
clientRequestId: string;
runId: string;
mode: 'prompt';
}>();
projectApi.list.mockResolvedValue({ projects: [project], activeProjectId: project.id });
projectApi.config.mockResolvedValue({ project, config });
projectApi.conversations.mockResolvedValue([conversation]);
conversationApi.events.mockResolvedValue(new FakeEventSource() as unknown as EventSource);
conversationApi.recover.mockResolvedValue(undefined);
conversationApi.submit.mockReturnValue(acceptanceFlight.promise);
attachmentApi.upload.mockImplementation(async (file: File) => ({
attachmentId: `attachment-${file.name}`,
mime: file.type,
byteLength: file.size,
}));
const { CodingChatPanel } = await import('@/pages/Chat/CodingChatPanel');
const { createLocalConversationSnapshot } = await import('@/pages/Chat/coding-chat-snapshot');
const { codingConversationStore } = await import('@/stores/coding-conversations');
conversationApi.snapshot.mockResolvedValue(createLocalConversationSnapshot(project.id, conversation));
render(<CodingChatPanel />);
const textbox = await screen.findByRole('textbox');
await waitFor(() => expect(codingConversationStore.getState().selectedConversationId)
.toBe(conversation.id));
const first = new File(['first'], 'first.png', { type: 'image/png' });
const next = new File(['next'], 'next.png', { type: 'image/png' });
fireEvent.change(screen.getByTestId('coding-file-attachment-input'), {
target: { files: [first] },
});
fireEvent.click(await screen.findByRole('button', { name: '发送' }));
await waitFor(() => expect(conversationApi.submit).toHaveBeenCalledOnce());
expect(screen.getByTestId('coding-file-attachment-input')).toBeDisabled();
fireEvent.paste(textbox, { clipboardData: { files: [next] } });
expect(screen.queryByAltText('next.png')).not.toBeInTheDocument();
expect(attachmentApi.upload).toHaveBeenCalledOnce();
acceptanceFlight.resolve({
accepted: true,
conversationId: conversation.id,
clientRequestId: 'request-1',
runId: 'run-1',
mode: 'prompt',
});
await waitFor(() => expect(screen.getByTestId('coding-file-attachment-input')).toBeEnabled());
fireEvent.paste(textbox, { clipboardData: { files: [next] } });
expect(await screen.findByAltText('next.png')).toBeInTheDocument();
expect(attachmentApi.upload).toHaveBeenCalledOnce();
});
it('keeps attachment validation failures local and reuses successful uploads on retry', async () => {
projectApi.list.mockResolvedValue({ projects: [project], activeProjectId: project.id });
projectApi.config.mockResolvedValue({ project, config });
projectApi.conversations.mockResolvedValue([conversation]);
conversationApi.events.mockResolvedValue(new FakeEventSource() as unknown as EventSource);
conversationApi.recover.mockResolvedValue(undefined);
conversationApi.submit.mockImplementation(async (input: {
conversationId: string;
clientRequestId: string;
mode: 'prompt';
}) => ({
accepted: true,
conversationId: input.conversationId,
clientRequestId: input.clientRequestId,
runId: 'run-1',
mode: input.mode,
}));
attachmentApi.upload.mockImplementation(async (file: File) => {
if (file.name === 'invalid.png') throw new Error('图片附件无效。');
return {
attachmentId: `attachment-${file.name}`,
mime: file.type,
byteLength: file.size,
};
});
const { CodingChatPanel } = await import('@/pages/Chat/CodingChatPanel');
const { createLocalConversationSnapshot } = await import('@/pages/Chat/coding-chat-snapshot');
const { codingConversationStore } = await import('@/stores/coding-conversations');
conversationApi.snapshot.mockResolvedValue(createLocalConversationSnapshot(project.id, conversation));
render(<CodingChatPanel />);
await screen.findByRole('textbox');
await waitFor(() => expect(codingConversationStore.getState().selectedConversationId)
.toBe(conversation.id));
const valid = new File(['valid'], 'valid.png', { type: 'image/png' });
const invalid = new File(['invalid'], 'invalid.png', { type: 'image/png' });
fireEvent.change(screen.getByTestId('coding-file-attachment-input'), {
target: { files: [valid, invalid] },
});
fireEvent.click(await screen.findByRole('button', { name: '发送' }));
expect(await screen.findByText('图片附件无效。')).toBeInTheDocument();
expect(codingConversationStore.getState().entriesByConversationId[conversation.id]?.error)
.toBeNull();
expect(screen.queryByRole('button', { name: '重试' })).not.toBeInTheDocument();
expect(screen.getByRole('button', { name: '发送' })).toBeEnabled();
fireEvent.click(screen.getByRole('button', { name: '移除图片 invalid.png' }));
await waitFor(() => expect(screen.queryByText('图片附件无效。')).not.toBeInTheDocument());
fireEvent.click(screen.getByRole('button', { name: '发送' }));
await waitFor(() => expect(conversationApi.submit).toHaveBeenCalledOnce());
expect(attachmentApi.upload.mock.calls.filter(([file]) => file.name === 'valid.png'))
.toHaveLength(1);
expect(attachmentApi.upload).toHaveBeenCalledTimes(2);
expect(conversationApi.recover).not.toHaveBeenCalled();
});
it('shows the 202 acceptance and preserves Enter versus Shift+Enter behavior', async () => {
const { CodingComposer } = await import('@/pages/Chat/CodingComposer');
const onSubmit = vi.fn();

View File

@@ -429,6 +429,55 @@ describe('coding Conversation store', () => {
.toMatchObject({ id: 'node-1', status: 'error' });
});
it('keeps attachment preparation failures out of runtime recovery and permits a direct retry', async () => {
const recover = vi.fn(async () => undefined);
const submitPrompt = vi.fn(async (input) => acceptance(
input.conversationId,
input.clientRequestId,
));
const store = createCodingConversationStore({
getSnapshot: vi.fn(),
openEvents: vi.fn(),
submitPrompt,
recover,
createId: ids(),
});
store.getState().applySnapshotEvent(snapshotEvent(snapshot('conversation-a')));
store.getState().setDraft('conversation-a', 'Inspect the image');
await expect(store.getState().submitPrompt({
conversationId: 'conversation-a',
mode: 'prompt',
prepareAttachments: async () => {
throw new AppError('VALIDATION', '图片附件无效。', undefined, {
backendCode: 'CODING_ATTACHMENT_INVALID',
});
},
})).rejects.toThrow('图片附件无效。');
expect(store.getState().entriesByConversationId['conversation-a']).toMatchObject({
loadState: 'live',
error: null,
});
expect(store.getState().draftsByConversationId['conversation-a'].text)
.toBe('Inspect the image');
expect(store.getState().requestsByConversationId['conversation-a']['request-1'])
.toMatchObject({ status: 'rejected', errorCode: 'CODING_ATTACHMENT_INVALID' });
expect(submitPrompt).not.toHaveBeenCalled();
expect(recover).not.toHaveBeenCalled();
await store.getState().submitPrompt({
conversationId: 'conversation-a',
mode: 'prompt',
prepareAttachments: async () => [{
attachmentId: 'attachment-valid',
mime: 'image/png',
previewUrl: 'blob:valid',
}],
});
expect(submitPrompt).toHaveBeenCalledOnce();
});
it('keeps an uncertain optimistic request recoverable without replaying it on reconnect', async () => {
const source = new FakeEventSource();
const submitPrompt = vi.fn(async () => {