mirror of
https://github.com/coder/coder.git
synced 2026-09-21 20:51:01 +08:00
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.
This commit is contained in:
+51
-3
@@ -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]);
|
||||
},
|
||||
};
|
||||
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -5,12 +5,12 @@ export type UserInlineRenderBlock =
|
||||
| Extract<RenderBlock, { type: "response" }>
|
||||
| Extract<RenderBlock, { type: "file-reference" }>;
|
||||
|
||||
type UserFileRenderBlock = Extract<RenderBlock, { type: "file" }>;
|
||||
type FileRenderBlock = Extract<RenderBlock, { type: "file" }>;
|
||||
|
||||
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 ||
|
||||
|
||||
Reference in New Issue
Block a user