From 524bca4c876d5893b3cc5fd3612ce78a95162443 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 17 Mar 2026 14:00:31 +0200 Subject: [PATCH] fix(site/src/pages/AgentsPage): fix chat image paste bugs and refactor queued message display (#22735) handleSubmit (triggered via Enter key) didn't check isUploading, so messages could be sent while an image upload was still in progress. The send button was correctly disabled via canSend, but the keyboard shortcut bypassed that guard. QueuedMessagesList used untyped extraction helpers that fell through to JSON.stringify for attachment-only messages. Replace them with a single getQueuedMessageInfo function using typed ChatMessagePart access. Show an attachment badge (ImageIcon + count) for file parts, and use a consistent "[Queued message]" placeholder for all no-text situations. Editing a queued message with file attachments silently dropped all attachments because handleStartQueueEdit only accepted text. Thread file blocks from QueuedMessagesList through the edit callback into handleStartQueueEdit, which now calls setEditingFileBlocks. The existing useEffect in AgentDetailInput picks these up and populates the attachment UI. Also clear editingFileBlocks in handleCancelQueueEdit and handleSendFromInput. --- .../AgentsPage/AgentChatInput.stories.tsx | 28 ++++ site/src/pages/AgentsPage/AgentChatInput.tsx | 13 +- site/src/pages/AgentsPage/AgentDetail.tsx | 13 +- .../pages/AgentsPage/AgentDetailContent.tsx | 10 +- site/src/pages/AgentsPage/AgentDetailView.tsx | 10 +- .../AgentsPage/QueuedMessagesList.stories.tsx | 116 ++++++++++++- .../AgentsPage/QueuedMessagesList.test.ts | 154 +++++++++++++++++ .../pages/AgentsPage/QueuedMessagesList.tsx | 155 ++++++++---------- 8 files changed, 404 insertions(+), 95 deletions(-) create mode 100644 site/src/pages/AgentsPage/QueuedMessagesList.test.ts diff --git a/site/src/pages/AgentsPage/AgentChatInput.stories.tsx b/site/src/pages/AgentsPage/AgentChatInput.stories.tsx index 198c2da1c0..af73b2bbc9 100644 --- a/site/src/pages/AgentsPage/AgentChatInput.stories.tsx +++ b/site/src/pages/AgentsPage/AgentChatInput.stories.tsx @@ -216,6 +216,34 @@ export const WithUploadingAttachment: Story = { })(), }; +export const UploadingDisablesSend: Story = { + args: (() => { + const file = createMockFile("uploading.png", "image/png"); + return { + attachments: [file], + uploadStates: new Map([ + [file, { status: "uploading" }], + ]), + previewUrls: new Map([[file, TINY_PNG]]), + onAttach: fn(), + onRemoveAttachment: fn(), + initialValue: "Message with uploading image", + }; + })(), + play: async ({ canvasElement, args }) => { + const canvas = within(canvasElement); + // Send should be disabled while an upload is still in progress, + // even though the editor has text content. + const sendButton = canvas.getByRole("button", { name: "Send" }); + expect(sendButton).toBeDisabled(); + // Enter key should not trigger send while uploading. + const editor = canvas.getByRole("textbox"); + await userEvent.click(editor); + await userEvent.keyboard("{Enter}"); + expect(args.onSend).not.toHaveBeenCalled(); + }, +}; + export const WithAttachmentError: Story = { args: (() => { const file = createMockFile("broken.png", "image/png"); diff --git a/site/src/pages/AgentsPage/AgentChatInput.tsx b/site/src/pages/AgentsPage/AgentChatInput.tsx index ec4a394d6e..8e7e6588b8 100644 --- a/site/src/pages/AgentsPage/AgentChatInput.tsx +++ b/site/src/pages/AgentsPage/AgentChatInput.tsx @@ -91,7 +91,15 @@ interface AgentChatInputProps { onPromoteQueuedMessage?: (id: number) => Promise | void; // Queue editing state, owned by the parent. editingQueuedMessageID?: number | null; - onStartQueueEdit?: (id: number, text: string) => void; + onStartQueueEdit?: ( + id: number, + text: string, + fileBlocks: readonly { + mediaType: string; + data?: string; + fileId?: string; + }[], + ) => void; onCancelQueueEdit?: () => void; // History editing state, owned by the parent. isEditingHistoryMessage?: boolean; @@ -496,6 +504,7 @@ export const AgentChatInput = memo( !hasFileReferences && !isDisabled && !isLoading && + !isUploading && queuedMessages.length > 0 && onPromoteQueuedMessage ) { @@ -507,6 +516,7 @@ export const AgentChatInput = memo( (!text && !hasUploadedAttachments && !hasFileReferences) || isDisabled || isLoading || + isUploading || !hasModelOptions ) { return; @@ -516,6 +526,7 @@ export const AgentChatInput = memo( }, [ isDisabled, isLoading, + isUploading, hasModelOptions, hasUploadedAttachments, hasFileReferences, diff --git a/site/src/pages/AgentsPage/AgentDetail.tsx b/site/src/pages/AgentsPage/AgentDetail.tsx index 9cfd557c3b..bb2badf4a6 100644 --- a/site/src/pages/AgentsPage/AgentDetail.tsx +++ b/site/src/pages/AgentsPage/AgentDetail.tsx @@ -143,13 +143,22 @@ export function useConversationEditingState(deps: { >(null); const handleStartQueueEdit = useCallback( - (id: number, text: string) => { + ( + id: number, + text: string, + fileBlocks: readonly { + mediaType: string; + data?: string; + fileId?: string; + }[], + ) => { setDraftBeforeQueueEdit((prev) => editingQueuedMessageID === null ? inputValueRef.current : prev, ); setEditingQueuedMessageID(id); setEditorInitialValue(text); inputValueRef.current = text; + setEditingFileBlocks(fileBlocks); }, [editingQueuedMessageID, inputValueRef], ); @@ -159,6 +168,7 @@ export function useConversationEditingState(deps: { inputValueRef.current = draftBeforeQueueEdit ?? ""; setEditingQueuedMessageID(null); setDraftBeforeQueueEdit(null); + setEditingFileBlocks([]); }, [draftBeforeQueueEdit, inputValueRef]); // Wraps the parent onSend to clear local input/editing state @@ -185,6 +195,7 @@ export function useConversationEditingState(deps: { if (queueEditID !== null) { setEditingQueuedMessageID(null); setDraftBeforeQueueEdit(null); + setEditingFileBlocks([]); void onDeleteQueuedMessage(queueEditID); } }, diff --git a/site/src/pages/AgentsPage/AgentDetailContent.tsx b/site/src/pages/AgentsPage/AgentDetailContent.tsx index 69bd5b93e4..b6685562a5 100644 --- a/site/src/pages/AgentsPage/AgentDetailContent.tsx +++ b/site/src/pages/AgentsPage/AgentDetailContent.tsx @@ -166,7 +166,15 @@ interface AgentDetailInputProps { initialValue?: string; onContentChange?: (content: string) => void; editingQueuedMessageID: number | null; - onStartQueueEdit: (id: number, text: string) => void; + onStartQueueEdit: ( + id: number, + text: string, + fileBlocks: readonly { + mediaType: string; + data?: string; + fileId?: string; + }[], + ) => void; onCancelQueueEdit: () => void; isEditingHistoryMessage: boolean; onCancelHistoryEdit: () => void; diff --git a/site/src/pages/AgentsPage/AgentDetailView.tsx b/site/src/pages/AgentsPage/AgentDetailView.tsx index 6d30c5884f..1093afb650 100644 --- a/site/src/pages/AgentsPage/AgentDetailView.tsx +++ b/site/src/pages/AgentsPage/AgentDetailView.tsx @@ -47,7 +47,15 @@ interface EditingState { ) => void; handleCancelHistoryEdit: () => void; editingQueuedMessageID: number | null; - handleStartQueueEdit: (id: number, text: string) => void; + handleStartQueueEdit: ( + id: number, + text: string, + fileBlocks: readonly { + mediaType: string; + data?: string; + fileId?: string; + }[], + ) => void; handleCancelQueueEdit: () => void; handleSendFromInput: (message: string, fileIds?: string[]) => void; handleContentChange: (content: string) => void; diff --git a/site/src/pages/AgentsPage/QueuedMessagesList.stories.tsx b/site/src/pages/AgentsPage/QueuedMessagesList.stories.tsx index 4d7cf1c502..d2854813e8 100644 --- a/site/src/pages/AgentsPage/QueuedMessagesList.stories.tsx +++ b/site/src/pages/AgentsPage/QueuedMessagesList.stories.tsx @@ -1,6 +1,6 @@ import type { Meta, StoryObj } from "@storybook/react-vite"; import type { ChatQueuedMessage } from "api/typesGenerated"; -import { fn } from "storybook/test"; +import { expect, fn, userEvent, within } from "storybook/test"; import { QueuedMessagesList } from "./QueuedMessagesList"; // Helper to build a ChatQueuedMessage with minimal boilerplate. @@ -67,11 +67,10 @@ export const MixedContentTypes: Story = { messages: [ // Typed text content. makeMessage(1, textContent("Plain text content")), - // Legacy serialized payload in a text field. - makeMessage( - 2, - textContent('[{"type":"text","data":{"text":"legacy payload"}}]'), - ), + // Attachment-only message falls back to the generic label. + makeMessage(2, [ + { type: "file", file_id: "img-1", media_type: "image/png" }, + ] as ChatQueuedMessage["content"]), // Empty content falls back to the generic label. makeMessage(3, [] as ChatQueuedMessage["content"]), ], @@ -101,3 +100,108 @@ export const LongMessageText: Story = { ], }, }; + +// Multi-line text is truncated to the first line with an ellipsis appended. +export const MultiLineTextTruncation: Story = { + args: { + messages: [ + makeMessage( + 1, + textContent( + "First line of the message\nSecond line that should be hidden", + ), + ), + ], + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + // The first line and ellipsis should be visible in the same span. + const textSpan = canvas.getByText(/First line of the message…/); + expect(textSpan).toBeInTheDocument(); + // The second line should not appear anywhere. + expect(canvas.queryByText(/Second line/)).not.toBeInTheDocument(); + }, +}; + +// A message with both text and a file attachment shows the ImageIcon badge. +export const WithAttachments: Story = { + args: { + messages: [ + makeMessage(1, [ + { type: "text", text: "Check this screenshot" }, + { type: "file", file_id: "abc-123", media_type: "image/png" }, + ] as ChatQueuedMessage["content"]), + ], + }, +}; + +// A message with only file attachments and no text displays a count label. +export const AttachmentsOnly: Story = { + args: { + messages: [ + makeMessage(1, [ + { type: "file", file_id: "img-1", media_type: "image/png" }, + { type: "file", file_id: "img-2", media_type: "image/jpeg" }, + ] as ChatQueuedMessage["content"]), + ], + }, +}; + +// Clicking Edit on a message with attachments passes file blocks to onEdit. +export const EditPassesFileBlocks: Story = { + args: { + onEdit: fn(), + messages: [ + makeMessage(1, [ + { type: "text", text: "Check this screenshot" }, + { type: "file", file_id: "abc-123", media_type: "image/png" }, + ] as ChatQueuedMessage["content"]), + ], + }, + play: async ({ canvasElement, args }) => { + const canvas = within(canvasElement); + const editButton = canvas.getByRole("button", { name: "Edit" }); + await userEvent.click(editButton); + expect(args.onEdit).toHaveBeenCalledWith(1, "Check this screenshot", [ + { mediaType: "image/png", fileId: "abc-123" }, + ]); + }, +}; + +// Clicking Edit on an attachment-only message passes file blocks with empty text. +export const EditAttachmentOnlyMessage: Story = { + args: { + onEdit: fn(), + messages: [ + makeMessage(1, [ + { type: "file", file_id: "img-1", media_type: "image/png" }, + { type: "file", file_id: "img-2", media_type: "image/jpeg" }, + ] as ChatQueuedMessage["content"]), + ], + }, + play: async ({ canvasElement, args }) => { + const canvas = within(canvasElement); + const editButton = canvas.getByRole("button", { name: "Edit" }); + await userEvent.click(editButton); + expect(args.onEdit).toHaveBeenCalledWith(1, "", [ + { mediaType: "image/png", fileId: "img-1" }, + { mediaType: "image/jpeg", fileId: "img-2" }, + ]); + }, +}; + +// A mixed queue with text-only, text+attachment, and attachment-only messages. +export const MixedQueueWithAttachments: Story = { + args: { + messages: [ + makeMessage(1, textContent("Run the linter")), + makeMessage(2, [ + { type: "text", text: "Fix this layout bug" }, + { type: "file", file_id: "img-a", media_type: "image/png" }, + ] as ChatQueuedMessage["content"]), + makeMessage(3, [ + { type: "file", file_id: "img-b", media_type: "image/png" }, + ] as ChatQueuedMessage["content"]), + ], + }, +}; diff --git a/site/src/pages/AgentsPage/QueuedMessagesList.test.ts b/site/src/pages/AgentsPage/QueuedMessagesList.test.ts new file mode 100644 index 0000000000..1695f44925 --- /dev/null +++ b/site/src/pages/AgentsPage/QueuedMessagesList.test.ts @@ -0,0 +1,154 @@ +import type { ChatQueuedMessage } from "api/typesGenerated"; +import { describe, expect, it } from "vitest"; +import { getQueuedMessageInfo } from "./QueuedMessagesList"; + +const makeMessage = ( + content: ChatQueuedMessage["content"], +): ChatQueuedMessage => ({ + id: 1, + chat_id: "c", + content, + created_at: "", +}); + +describe("getQueuedMessageInfo", () => { + it("returns text for a text-only message", () => { + const result = getQueuedMessageInfo( + makeMessage([{ type: "text", text: "hello" }]), + ); + expect(result).toEqual({ + displayText: "hello", + rawText: "hello", + attachmentCount: 0, + fileBlocks: [], + }); + }); + + it("preserves multi-line text", () => { + const result = getQueuedMessageInfo( + makeMessage([{ type: "text", text: "line1\nline2" }]), + ); + expect(result).toEqual({ + displayText: "line1\nline2", + rawText: "line1\nline2", + attachmentCount: 0, + fileBlocks: [], + }); + }); + + it("returns attachment label for a single file", () => { + const result = getQueuedMessageInfo( + makeMessage([{ type: "file", file_id: "a" }]), + ); + expect(result).toEqual({ + displayText: "[Queued message]", + rawText: "", + attachmentCount: 1, + fileBlocks: [{ mediaType: "application/octet-stream", fileId: "a" }], + }); + }); + + it("returns attachment label for multiple files", () => { + const result = getQueuedMessageInfo( + makeMessage([ + { type: "file", file_id: "a" }, + { type: "file", file_id: "b" }, + ]), + ); + expect(result).toEqual({ + displayText: "[Queued message]", + rawText: "", + attachmentCount: 2, + fileBlocks: [ + { mediaType: "application/octet-stream", fileId: "a" }, + { mediaType: "application/octet-stream", fileId: "b" }, + ], + }); + }); + + it("returns text with attachment count for text + file", () => { + const result = getQueuedMessageInfo( + makeMessage([ + { type: "text", text: "look" }, + { type: "file", file_id: "a" }, + ]), + ); + expect(result).toEqual({ + displayText: "look", + rawText: "look", + attachmentCount: 1, + fileBlocks: [{ mediaType: "application/octet-stream", fileId: "a" }], + }); + }); + + it("returns fallback for empty content", () => { + const result = getQueuedMessageInfo(makeMessage([])); + expect(result).toEqual({ + displayText: "[Queued message]", + rawText: "", + attachmentCount: 0, + fileBlocks: [], + }); + }); + + it("returns fallback for whitespace-only text", () => { + const result = getQueuedMessageInfo( + makeMessage([{ type: "text", text: " " }]), + ); + expect(result).toEqual({ + displayText: "[Queued message]", + rawText: "", + attachmentCount: 0, + fileBlocks: [], + }); + }); + + it("returns attachment label for whitespace text + file", () => { + const result = getQueuedMessageInfo( + makeMessage([ + { type: "text", text: " " }, + { type: "file", file_id: "a" }, + ]), + ); + expect(result).toEqual({ + displayText: "[Queued message]", + rawText: "", + attachmentCount: 1, + fileBlocks: [{ mediaType: "application/octet-stream", fileId: "a" }], + }); + }); + + it("joins multiple text parts with a space", () => { + const result = getQueuedMessageInfo( + makeMessage([ + { type: "text", text: "a" }, + { type: "text", text: "b" }, + ]), + ); + expect(result).toEqual({ + displayText: "a b", + rawText: "a b", + attachmentCount: 0, + fileBlocks: [], + }); + }); + + it("preserves media_type from file parts", () => { + const result = getQueuedMessageInfo( + makeMessage([ + { type: "text", text: "check this" }, + { type: "file", file_id: "img-1", media_type: "image/png" }, + { type: "file", file_id: "doc-2", media_type: "application/pdf" }, + ]), + ); + expect(result).toEqual({ + displayText: "check this", + rawText: "check this", + attachmentCount: 2, + fileBlocks: [ + { mediaType: "image/png", fileId: "img-1" }, + { mediaType: "application/pdf", fileId: "doc-2" }, + ], + }); + }); +}); diff --git a/site/src/pages/AgentsPage/QueuedMessagesList.tsx b/site/src/pages/AgentsPage/QueuedMessagesList.tsx index 3862c17fc9..a6da3fabc9 100644 --- a/site/src/pages/AgentsPage/QueuedMessagesList.tsx +++ b/site/src/pages/AgentsPage/QueuedMessagesList.tsx @@ -9,101 +9,67 @@ import { import { ArrowUpIcon, CornerDownLeftIcon, + ImageIcon, PencilIcon, Trash2Icon, } from "lucide-react"; import { type FC, useCallback, useEffect, useMemo, useState } from "react"; import { cn } from "utils/cn"; +interface FileBlock { + mediaType: string; + data?: string; + fileId?: string; +} + interface QueuedMessagesListProps { messages: readonly ChatQueuedMessage[]; onDelete: (id: number) => Promise | void; onPromote: (id: number) => Promise | void; - onEdit?: (id: number, text: string) => void; + onEdit?: (id: number, text: string, fileBlocks: readonly FileBlock[]) => void; editingMessageID?: number | null; className?: string; } -const asRecord = (value: unknown): Record | undefined => { - if (!value || typeof value !== "object" || Array.isArray(value)) { - return undefined; - } - return value as Record; -}; +interface QueuedMessageInfo { + displayText: string; + rawText: string; + attachmentCount: number; + fileBlocks: readonly FileBlock[]; +} -const extractBlockText = (value: unknown): string | undefined => { - if (typeof value === "string") { - return value; - } - const record = asRecord(value); - if (!record) { - return undefined; - } - if (typeof record.text === "string") { - return record.text; - } - const data = asRecord(record.data); - if (data && typeof data.text === "string") { - return data.text; - } - return undefined; -}; +export const getQueuedMessageInfo = ( + message: ChatQueuedMessage, +): QueuedMessageInfo => { + const { content } = message; + const fileBlocks: FileBlock[] = content + .filter((p) => p.type === "file") + .map((p) => ({ + mediaType: p.media_type ?? "application/octet-stream", + fileId: p.file_id, + data: p.data, + })); + const rawText = content + .filter((p) => p.type === "text") + .map((p) => p.text) + .filter((t): t is string => Boolean(t?.trim())) + .join(" ") + .trim(); -const extractQueuedContentText = (value: unknown): string => { - if (typeof value === "string") { - const trimmed = value.trim(); - if (trimmed === "") { - return ""; - } - if (trimmed.startsWith("[") || trimmed.startsWith("{")) { - try { - return extractQueuedContentText(JSON.parse(trimmed)); - } catch { - return value; - } - } - return value; + if (rawText) { + return { + displayText: rawText, + rawText, + attachmentCount: fileBlocks.length, + fileBlocks, + }; } - - if (Array.isArray(value)) { - const texts = value - .map(extractBlockText) - .filter((text): text is string => Boolean(text?.trim())); - if (texts.length > 0) { - return texts.join(" "); - } - try { - return JSON.stringify(value); - } catch { - return ""; - } - } - - const record = asRecord(value); - if (record) { - const text = extractBlockText(record); - if (text?.trim()) { - return text; - } - if ("content" in record) { - const nested = extractQueuedContentText(record.content); - if (nested.trim()) { - return nested; - } - } - try { - return JSON.stringify(record); - } catch { - return ""; - } - } - - return ""; -}; - -const getQueuedMessageText = (message: ChatQueuedMessage): string => { - const text = extractQueuedContentText(message.content).trim(); - return text || "Queued message"; + return { + displayText: "[Queued message]", + rawText: "", + attachmentCount: fileBlocks.length, + fileBlocks, + }; }; export const QueuedMessagesList: FC = ({ @@ -116,10 +82,17 @@ export const QueuedMessagesList: FC = ({ }) => { const items = useMemo( () => - messages.map((message) => ({ - id: message.id, - text: getQueuedMessageText(message), - })), + messages.map((message) => { + const { displayText, rawText, attachmentCount, fileBlocks } = + getQueuedMessageInfo(message); + return { + id: message.id, + displayText, + rawText, + attachmentCount, + fileBlocks, + }; + }), [messages], ); @@ -240,9 +213,19 @@ export const QueuedMessagesList: FC = ({ >
- {item.text.split("\n")[0]} - {item.text.includes("\n") ? "…" : ""} + {item.displayText.split("\n")[0]} + {item.displayText.includes("\n") ? "…" : ""} + {item.attachmentCount > 0 && ( + + + )} {isFirst && ( = ({ size="icon" aria-label="Edit" disabled={isBusy} - onClick={() => onEdit(item.id, item.text)} + onClick={() => + onEdit(item.id, item.rawText, item.fileBlocks) + } className="size-6 rounded text-content-secondary hover:bg-surface-tertiary hover:text-content-primary" >