From ba734f8b10ebaa075d7217820fc072ebedddd16a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 1 Apr 2026 18:27:27 +0300 Subject: [PATCH] fix(site/src/pages/AgentsPage): fix copy button toolbar regression and add missing story coverage (#23912) Move !isSavingMessage to the outer toolbar guard so the gradient container does not mount empty during save. Remove the now-redundant inner guard. Add flex to the assistant copy button wrapper div. The plain block wrapper with an inline-flex button created a line box whose height depended on the inherited non-integer line-height (14px * 1.625 = 22.75px strut). Sub-pixel rounding during hover repaints caused a 1px jitter. Making it a flex container eliminates the strut. Add behavioral assertions to UserMessageCopyButton: click edit and assert onEditUserMessage fires, click copy and assert writeText is called with the raw markdown. Add MultiAssistantTurnCopyButton regression story for the isLastAssistantMessage fix. Refs #23850 --- .../ConversationTimeline.stories.tsx | 99 ++++++++++++++++++- .../ChatConversation/ConversationTimeline.tsx | 75 +++++++------- 2 files changed, 136 insertions(+), 38 deletions(-) diff --git a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx index 6706bda66e..aa8df60931 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx @@ -585,7 +585,7 @@ export const UserMessageCopyButton: Story = { ]), onEditUserMessage: fn(), }, - play: async ({ canvasElement }) => { + play: async ({ args, canvasElement }) => { const canvas = within(canvasElement); // Force the hover-reveal toolbar visible for the screenshot. for (const el of canvasElement.querySelectorAll("[class]")) { @@ -604,6 +604,35 @@ export const UserMessageCopyButton: Story = { name: "Edit message", }); expect(editButton).toBeInTheDocument(); + + // Behavioral: clicking edit fires onEditUserMessage with the + // correct message ID and text. + await userEvent.click(editButton); + expect(args.onEditUserMessage).toHaveBeenCalledWith( + 1, + "Can you fix this bug?", + undefined, + ); + + // Behavioral: clicking copy writes the raw markdown to the + // clipboard. + const originalClipboard = navigator.clipboard; + const writeText = fn().mockResolvedValue(undefined); + Object.defineProperty(navigator, "clipboard", { + value: { writeText }, + writable: true, + configurable: true, + }); + try { + await userEvent.click(copyButton); + expect(writeText).toHaveBeenCalledWith("Can you fix this bug?"); + } finally { + Object.defineProperty(navigator, "clipboard", { + value: originalClipboard, + writable: true, + configurable: true, + }); + } }, }; @@ -736,3 +765,71 @@ export const CopyButtonWritesToClipboard: Story = { } }, }; + +/** + * Regression: copy button appears only on the last assistant message + * in a turn that includes tool calls. The isLastAssistantMessage + * computation must skip tool-role messages when finding turn + * boundaries. + */ +export const MultiAssistantTurnCopyButton: Story = { + args: { + ...defaultArgs, + parsedMessages: buildMessages([ + { + ...baseMessage, + id: 1, + role: "user", + content: [{ type: "text", text: "Help me refactor" }], + }, + { + ...baseMessage, + id: 2, + role: "assistant", + content: [ + { type: "text", text: "Let me check the code first." }, + { + type: "tool-call", + tool_call_id: "tool-1", + tool_name: "read_file", + args: { path: "main.go" }, + }, + ], + }, + { + ...baseMessage, + id: 3, + role: "tool", + content: [ + { + type: "tool-result", + tool_call_id: "tool-1", + result: { output: "package main" }, + }, + ], + }, + { + ...baseMessage, + id: 4, + role: "assistant", + content: [ + { type: "text", text: "Here is the **refactored** version." }, + ], + }, + ]), + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + // Only the last assistant message in the turn should have the + // copy button. The first assistant message (id=2) has text but + // should not show the button because a later assistant message + // (id=4) continues the turn. + const wrappers = canvas.getAllByTestId("assistant-copy-button"); + expect(wrappers).toHaveLength(1); + + const copyBtn = within(wrappers[0]).getByRole("button", { + name: "Copy message", + }); + expect(copyBtn).toBeInTheDocument(); + }, +}; diff --git a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx index 516d192953..1303edf3af 100644 --- a/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx +++ b/site/src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.tsx @@ -627,7 +627,10 @@ const ChatMessageItem = memo<{ mcpServers={mcpServers} afterResponseSlot={ hasCopyableContent && isLastAssistantMessage ? ( -
+
)} - {isUser && (hasCopyableContent || onEditUserMessage) && ( -
- {(hasCopyableContent || onEditUserMessage) && !isSavingMessage && ( - <> - {hasCopyableContent && ( - - )} - {onEditUserMessage && ( - - - - - Edit message - - )} - - )} -
- )} + {isUser && + !isSavingMessage && + (hasCopyableContent || onEditUserMessage) && ( +
+ {hasCopyableContent && ( + + )} + {onEditUserMessage && ( + + + + + Edit message + + )} +
+ )} {previewImage && (