From 38091f1d82c255c695fae5f321bf034ba2bf0fee Mon Sep 17 00:00:00 2001 From: Thomas Kosiewski Date: Tue, 12 May 2026 15:46:39 +0200 Subject: [PATCH] fix(site/src/pages/AgentsPage/components/ChatConversation): remove attachment copy actions (#25119) Messages with chat file attachments showed a `Copy message` action even though the action only copied `parsed.markdown` and omitted the attachment content. Hide the message copy action whenever a parsed message contains file attachments, and add regression coverage for both user and assistant attachment messages. Refs https://linear.app/codercom/issue/CODAGT-344/remove-or-fix-copy-buttons-on-file-attachments Generated by Coder Agents. --- .../ConversationTimeline.stories.tsx | 54 ++++++++++++++++- .../ChatConversation/messageHelpers.test.ts | 60 +++++++++++++++++++ .../ChatConversation/messageHelpers.ts | 17 +++--- 3 files changed, 119 insertions(+), 12 deletions(-) create mode 100644 site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.test.ts diff --git a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx index 66fd1cb059..f833496dfe 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx @@ -252,6 +252,20 @@ const findAttachmentTile = async ( return tile; }; +const expectNoCopyMessageButtonForElement = (element: HTMLElement) => { + const messageRow = element.closest( + '[data-role="user"], [data-role="assistant"]', + ); + expect(messageRow).not.toBeNull(); + const messageWrapper = messageRow?.parentElement; + expect(messageWrapper).not.toBeNull(); + expect( + within(messageWrapper as HTMLElement).queryByRole("button", { + name: "Copy message", + }), + ).not.toBeInTheDocument(); +}; + const hoverAndExpectTooltip = async ( element: HTMLElement, text: RegExp | string, @@ -366,6 +380,7 @@ export const UserMessageWithSingleImage: Story = { const canvas = within(canvasElement); const images = canvas.getAllByRole("img", { name: "Attached image" }); expect(images).toHaveLength(1); + expectNoCopyMessageButtonForElement(images[0]); }, }; @@ -403,6 +418,7 @@ export const UserMessageWithMultipleImages: Story = { const canvas = within(canvasElement); const images = canvas.getAllByRole("img", { name: "Attached image" }); expect(images).toHaveLength(3); + expectNoCopyMessageButtonForElement(images[0]); }, }; @@ -423,6 +439,7 @@ export const UserMessageWithFileIdImage: Story = { "src", getChatFileURL("storybook-test-image"), ); + expectNoCopyMessageButtonForElement(images[0]); }, }; @@ -443,6 +460,7 @@ export const UserMessageWithExpiredImage: Story = { expect( canvas.queryByRole("button", { name: "View Attached image" }), ).not.toBeInTheDocument(); + expectNoCopyMessageButtonForElement(expiredTile); // The tooltip explains the retention policy generically so the // copy survives any operator-chosen retention window. @@ -482,6 +500,9 @@ export const UserMessageWithRepeatedExpiredImage: Story = { expect( canvas.queryByRole("button", { name: "View Attached image" }), ).not.toBeInTheDocument(); + for (const tile of canvas.getAllByRole("img", { name: "Image expired" })) { + expectNoCopyMessageButtonForElement(tile); + } }, }; @@ -523,6 +544,7 @@ export const UserMessageWithRepeatedFailedImage: Story = { return t; }); for (const tile of tiles) { + expectNoCopyMessageButtonForElement(tile); await hoverAndExpectTooltip(tile, FAILED_ATTACHMENT_API_MESSAGE); } }, @@ -540,11 +562,12 @@ export const UserMessageWithFailedRemoteImage: Story = { const canvas = within(canvasElement); const image = canvas.getByRole("img", { name: "Attached image" }); fireEvent.error(image); - await findAttachmentTile(canvas, "Image failed to load"); + const failedTile = await findAttachmentTile(canvas, "Image failed to load"); expect(canvas.getByText("This image failed to load")).toBeInTheDocument(); expect( canvas.queryByRole("button", { name: "View Attached image" }), ).not.toBeInTheDocument(); + expectNoCopyMessageButtonForElement(failedTile); // When the probe returns a structured error body, the tooltip // surfaces the API's message so the viewer has something @@ -572,7 +595,8 @@ export const UserMessageWithUndisplayableRemoteImage: Story = { const canvas = within(canvasElement); const image = canvas.getByRole("img", { name: "Attached image" }); fireEvent.error(image); - await findAttachmentTile(canvas, "Image failed to load"); + const failedTile = await findAttachmentTile(canvas, "Image failed to load"); + expectNoCopyMessageButtonForElement(failedTile); await hoverAndExpectTooltip( await waitForTooltipWrappedAttachmentTile(canvas, "Image failed to load"), UNDISPLAYABLE_REMOTE_ATTACHMENT_MESSAGE, @@ -592,13 +616,14 @@ export const UserMessageWithInvalidInlineImage: Story = { const canvas = within(canvasElement); const image = canvas.getByRole("img", { name: "Attached image" }); fireEvent.error(image); - await findAttachmentTile(canvas, "Image failed to load"); + const failedTile = await findAttachmentTile(canvas, "Image failed to load"); expect( canvas.getByText("Inline image data is corrupt"), ).toBeInTheDocument(); expect( canvas.queryByRole("button", { name: "View Attached image" }), ).not.toBeInTheDocument(); + expectNoCopyMessageButtonForElement(failedTile); }, }; @@ -616,6 +641,9 @@ export const UserMessageWithTextAttachment: Story = { }); expect(textButton).toBeInTheDocument(); expect(textButton).toHaveTextContent(/Pasted text/i); + expect( + canvas.queryByRole("button", { name: "Copy message" }), + ).not.toBeInTheDocument(); await userEvent.click(textButton); expect( await canvas.findByText(/Quarterly revenue increased 18%/i), @@ -649,6 +677,9 @@ export const UserMessageWithJSONAttachment: Story = { name: "View report.json", }); expect(textButton).toHaveTextContent("report.json"); + expect( + canvas.queryByRole("button", { name: "Copy message" }), + ).not.toBeInTheDocument(); await userEvent.click(textButton); expect(await canvas.findByText(/"status":"ok"/i)).toBeInTheDocument(); }, @@ -684,6 +715,9 @@ export const UserMessageWithDownloadableFile: Story = { "/api/experimental/chats/files/storybook-user-deployment-report", ); expect(canvas.getByText("deployment-report.pdf")).toBeInTheDocument(); + expect( + canvas.queryByRole("button", { name: "Copy message" }), + ).not.toBeInTheDocument(); }, }; @@ -705,6 +739,7 @@ export const UserMessageWithMultipleTextAttachments: Story = { name: "View text attachment", }); expect(textButtons).toHaveLength(3); + expectNoCopyMessageButtonForElement(textButtons[0]); }, }; @@ -720,6 +755,7 @@ export const UserMessageWithTextAttachmentOnly: Story = { name: "View text attachment", }); expect(textButton).toHaveTextContent(/Pasted text/i); + expectNoCopyMessageButtonForElement(textButton); await userEvent.click(textButton); expect( await canvas.findByText(/Runbook note: restart the worker/i), @@ -739,6 +775,7 @@ export const UserMessageWithExpiredTextAttachment: Story = { const textButton = await canvas.findByRole("button", { name: "View text attachment", }); + expectNoCopyMessageButtonForElement(textButton); await userEvent.click(textButton); const expiredTile = await findAttachmentTile(canvas, "Attachment expired"); expect( @@ -767,6 +804,7 @@ export const UserMessageWithFailedTextAttachment: Story = { const textButton = await canvas.findByRole("button", { name: "View text attachment", }); + expectNoCopyMessageButtonForElement(textButton); await userEvent.click(textButton); await findAttachmentTile(canvas, "Attachment failed to load"); expect( @@ -813,6 +851,7 @@ export const UserMessageWithInlineTextAttachment: Story = { name: "View text attachment", }); expect(textButton).toHaveTextContent(/Pasted text/i); + expectNoCopyMessageButtonForElement(textButton); await userEvent.click(textButton); expect( await canvas.findByText(/Inline deployment note/i), @@ -850,6 +889,7 @@ export const UserMessageWithFailedTextAttachmentNonJSONBody: Story = { const textButton = await canvas.findByRole("button", { name: "View preview.txt", }); + expectNoCopyMessageButtonForElement(textButton); await userEvent.click(textButton); await findAttachmentTile(canvas, "Attachment failed to load"); expect( @@ -878,6 +918,7 @@ export const UserMessageWithMixedAttachments: Story = { name: "View text attachment", }); expect(textButtons).toHaveLength(1); + expectNoCopyMessageButtonForElement(images[0]); }, }; @@ -934,6 +975,9 @@ export const AssistantMessageWithImage: Story = { expect( canvas.queryByRole("link", { name: "Download generated-image.png" }), ).not.toBeInTheDocument(); + expect( + canvas.queryByRole("button", { name: "Copy message" }), + ).not.toBeInTheDocument(); const viewButton = canvas.getByRole("button", { name: "View generated-image.png", }); @@ -974,6 +1018,9 @@ export const AssistantMessageWithUnnamedDownloadableFile: Story = { expect(downloadLink).toBeInTheDocument(); expect(downloadLink).toHaveAttribute("download", "attachment.pdf"); expect(canvas.getByText("Attached file")).toBeInTheDocument(); + expect( + canvas.queryByRole("button", { name: "Copy message" }), + ).not.toBeInTheDocument(); }, }; @@ -1009,6 +1056,7 @@ export const UserMessageWithImagesAndFileRefs: Story = { const images = canvas.getAllByRole("img", { name: "Attached image" }); expect(images).toHaveLength(1); expect(canvas.getByText(/main\.go/)).toBeInTheDocument(); + expectNoCopyMessageButtonForElement(images[0]); }, }; diff --git a/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.test.ts b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.test.ts new file mode 100644 index 0000000000..b2127454c8 --- /dev/null +++ b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it } from "vitest"; +import type { ChatMessage, ChatMessagePart } from "#/api/typesGenerated"; +import { deriveMessageDisplayState } from "./messageHelpers"; +import { parseMessageContent } from "./messageParsing"; + +const buildMessage = ( + content: ChatMessagePart[], + role: "user" | "assistant" = "user", +): ChatMessage => ({ + id: 1, + chat_id: "chat-1", + created_at: "2026-05-11T00:00:00.000Z", + role, + content, +}); + +const getDisplayState = (message: ChatMessage) => + deriveMessageDisplayState({ + message, + parsed: parseMessageContent(message.content), + hideActions: false, + }); + +describe("deriveMessageDisplayState", () => { + it("marks text-only user messages as copyable", () => { + const message = buildMessage([{ type: "text", text: "Copy this" }]); + + expect(getDisplayState(message).hasCopyableContent).toBe(true); + }); + + it("marks text-only assistant messages as copyable", () => { + const message = buildMessage( + [{ type: "text", text: "Here is my answer." }], + "assistant", + ); + + expect(getDisplayState(message).hasCopyableContent).toBe(true); + }); + + it("does not mark user messages with file attachments as copyable", () => { + const message = buildMessage([ + { type: "text", text: "Copy should not omit this file." }, + { type: "file", media_type: "text/plain", file_id: "file-1" }, + ]); + + expect(getDisplayState(message).hasCopyableContent).toBe(false); + }); + + it("does not mark assistant messages with file attachments as copyable", () => { + const message = buildMessage( + [ + { type: "text", text: "Generated file attached." }, + { type: "file", media_type: "image/png", file_id: "image-1" }, + ], + "assistant", + ); + + expect(getDisplayState(message).hasCopyableContent).toBe(false); + }); +}); diff --git a/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts index d789aa7ec2..776b2bc01d 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts +++ b/site/src/pages/AgentsPage/components/ChatConversation/messageHelpers.ts @@ -5,12 +5,12 @@ export type UserInlineRenderBlock = | Extract | Extract; -type UserFileRenderBlock = Extract; +type FileRenderBlock = Extract; export type MessageDisplayState = { shouldHide: boolean; userInlineContent: UserInlineRenderBlock[]; - userFileBlocks: UserFileRenderBlock[]; + userFileBlocks: FileRenderBlock[]; hasUserMessageBody: boolean; hasFileBlocks: boolean; hasCopyableContent: boolean; @@ -22,9 +22,8 @@ const isUserInlineRenderBlock = ( ): block is UserInlineRenderBlock => block.type === "response" || block.type === "file-reference"; -const isUserFileRenderBlock = ( - block: RenderBlock, -): block is UserFileRenderBlock => block.type === "file"; +const isFileRenderBlock = (block: RenderBlock): block is FileRenderBlock => + block.type === "file"; const isProviderToolResultOnlyMessage = ( parts: readonly TypesGen.ChatMessagePart[], @@ -53,13 +52,13 @@ export const deriveMessageDisplayState = ({ const userInlineContent = isUser ? parsed.blocks.filter(isUserInlineRenderBlock) : []; - const userFileBlocks = isUser - ? parsed.blocks.filter(isUserFileRenderBlock) - : []; + const userFileBlocks = isUser ? parsed.blocks.filter(isFileRenderBlock) : []; + const hasFileAttachments = parsed.blocks.some(isFileRenderBlock); const hasUserMessageBody = userInlineContent.length > 0 || Boolean(parsed.markdown.trim()); const hasFileBlocks = userFileBlocks.length > 0; - const hasCopyableContent = Boolean(parsed.markdown.trim()); + const hasCopyableContent = + Boolean(parsed.markdown.trim()) && !hasFileAttachments; const hasRenderableContent = parsed.blocks.length > 0 || parsed.tools.length > 0 ||