From 3cf867f84aa32d2febf7a26dc7e52be6beb8a2ac Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Wed, 27 May 2026 18:10:10 +0200 Subject: [PATCH] fix(site/src/pages/AgentsPage): make other-user chats read-only (#25736) Other-user agent chats showed a banner that implied prompts would run as the owner, but submitting from that view is forbidden. This updates the banner to identify the chat owner and makes chats owned by another user read-only in the UI by disabling the composer and hiding inline send or edit follow-up actions. > Mux working on behalf of Mike. --- .../AgentsPage/AgentChatPage.stories.tsx | 141 ++++++++++-------- site/src/pages/AgentsPage/AgentChatPage.tsx | 39 ++--- .../AgentsPage/AgentChatPageView.stories.tsx | 137 ++++++++++------- .../pages/AgentsPage/AgentChatPageView.tsx | 41 +++-- 4 files changed, 197 insertions(+), 161 deletions(-) diff --git a/site/src/pages/AgentsPage/AgentChatPage.stories.tsx b/site/src/pages/AgentsPage/AgentChatPage.stories.tsx index 6fd809f9fd..0945ca1fc9 100644 --- a/site/src/pages/AgentsPage/AgentChatPage.stories.tsx +++ b/site/src/pages/AgentsPage/AgentChatPage.stories.tsx @@ -190,15 +190,13 @@ const extractPromptsFromMessages = ( return prompts; }; type ChatAuthorizationFixture = { - action: "share" | "update"; + action: "share"; allowed: boolean; }; const buildChatAuthorizationQuery = ( chat: Pick, - checks: Partial< - Record<"canShareChat" | "canUpdateChat", ChatAuthorizationFixture> - >, + checks: Partial>, ) => { const authorizationChecks: TypesGen.AuthorizationRequest["checks"] = {}; const authorizationResponse: TypesGen.AuthorizationResponse = {}; @@ -1180,9 +1178,6 @@ export const WithMessageHistory: Story = { await canvas.findByText("Markdown rendering showcase"), ).toBeVisible(); await waitFor(() => { - expect( - canvas.queryByText(/^This is not your chat/), - ).not.toBeInTheDocument(); expect( canvas.queryByText(/^This chat is owned by/), ).not.toBeInTheDocument(); @@ -1246,87 +1241,106 @@ export const Loading: Story = { }, }; -export const AdminViewingOtherUserChat: Story = { +export const OtherUserChatReadOnly: Story = { parameters: { - queries: [ - ...buildQueries( - { - id: CHAT_ID, - ...baseChatFields, - owner_id: "other-user-id", - owner_username: "OtherUser", - owner_name: "Other User", - title: "Other user's chat", - status: "completed", - }, - { messages: [], queued_messages: [], has_more: false }, - { diffUrl: undefined }, - ), - buildChatAuthorizationQuery( - { - owner_id: "other-user-id", - organization_id: baseChatFields.organization_id, - }, - { - canUpdateChat: { action: "update", allowed: true }, - canShareChat: { action: "share", allowed: false }, - }, - ), - ], + queries: buildQueries( + { + id: CHAT_ID, + ...baseChatFields, + owner_id: "other-user-id", + owner_username: "OtherUser", + owner_name: "Other User", + title: "Other user's chat", + status: "completed", + }, + { messages: [], queued_messages: [], has_more: false }, + { diffUrl: undefined }, + ), }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); const banner = await canvas.findByText( - "This is not your chat. Prompting here will use Other User's identity.", + "This chat is owned by Other User. It is read-only.", ); expect(banner).toBeVisible(); expect(banner).toHaveAttribute("role", "status"); expect(canvas.getByRole("textbox")).toHaveAttribute( "aria-disabled", - "false", + "true", ); }, }; -export const SharedReadOnlyChat: Story = { +export const OtherUserChatWithMessages: Story = { parameters: { - queries: [ - ...buildQueries( - { - id: CHAT_ID, - ...baseChatFields, - owner_id: "other-user-id", - owner_username: "OtherUser", - owner_name: "Other User", - title: "Shared read-only chat", - status: "completed", - }, - { messages: [], queued_messages: [], has_more: false }, - { diffUrl: undefined }, - ), - buildChatAuthorizationQuery( - { - owner_id: "other-user-id", - organization_id: baseChatFields.organization_id, - }, - { - canUpdateChat: { action: "update", allowed: false }, - canShareChat: { action: "share", allowed: false }, - }, - ), - ], + queries: buildQueries( + { + id: CHAT_ID, + ...baseChatFields, + owner_id: "other-user-id", + owner_username: "OtherUser", + owner_name: "Other User", + title: "Other user's chat with messages", + status: "completed", + }, + { + messages: [ + { + id: 1, + chat_id: CHAT_ID, + created_at: "2026-02-18T00:00:01.000Z", + role: "user", + content: [{ type: "text", text: "Please review this plan." }], + }, + { + id: 2, + chat_id: CHAT_ID, + created_at: "2026-02-18T00:00:02.000Z", + role: "assistant", + content: [ + { type: "text", text: "I prepared a plan." }, + { + type: "tool-call", + tool_call_id: "other-user-plan", + tool_name: "propose_plan", + args: { path: "/home/coder/PLAN.md" }, + }, + { + type: "tool-result", + tool_call_id: "other-user-plan", + tool_name: "propose_plan", + result: { + file_id: "other-user-plan-file", + content: "# Plan\n\n1. Keep this chat read-only.", + }, + }, + ], + }, + ] as TypesGen.ChatMessage[], + queued_messages: [], + has_more: false, + }, + { diffUrl: undefined }, + ), }, play: async ({ canvasElement }) => { const canvas = within(canvasElement); expect( await canvas.findByText( - "This chat is owned by Other User. You have read-only access.", + "This chat is owned by Other User. It is read-only.", ), ).toBeVisible(); + expect(await canvas.findByText("Please review this plan.")).toBeVisible(); expect(canvas.getByRole("textbox")).toHaveAttribute( "aria-disabled", "true", ); + expect( + canvas.queryByRole("button", { name: "Edit message" }), + ).not.toBeInTheDocument(); + expect( + canvas.queryByRole("button", { name: "Implement plan" }), + ).not.toBeInTheDocument(); }, }; @@ -1352,9 +1366,6 @@ export const ArchivedOtherUserChat: Story = { expect( await canvas.findByText("This agent has been archived and is read-only."), ).toBeVisible(); - expect( - canvas.queryByText(/^This is not your chat/), - ).not.toBeInTheDocument(); expect( canvas.queryByText(/^This chat is owned by/), ).not.toBeInTheDocument(); diff --git a/site/src/pages/AgentsPage/AgentChatPage.tsx b/site/src/pages/AgentsPage/AgentChatPage.tsx index 9a73a122cf..8eca4e5f7d 100644 --- a/site/src/pages/AgentsPage/AgentChatPage.tsx +++ b/site/src/pages/AgentsPage/AgentChatPage.tsx @@ -845,8 +845,6 @@ const AgentChatPage: FC = () => { chatRecord !== undefined && currentUser.id !== chatRecord.owner_id; const isRootChat = chatRecord !== undefined && getParentChatID(chatRecord) === undefined; - const shouldCheckCanUpdateOtherUserChat = - isViewerNotOwner && !isArchived && chatRecord !== undefined; const shouldCheckCanShareChat = isRootChat; const chatAuthorizationObject = chatRecord !== undefined @@ -857,15 +855,6 @@ const AgentChatPage: FC = () => { } : undefined; const chatAuthorizationChecks: TypesGen.AuthorizationRequest["checks"] = {}; - if ( - chatAuthorizationObject !== undefined && - shouldCheckCanUpdateOtherUserChat - ) { - chatAuthorizationChecks.canUpdateChat = { - object: chatAuthorizationObject, - action: "update", - }; - } if (chatAuthorizationObject !== undefined && shouldCheckCanShareChat) { chatAuthorizationChecks.canShareChat = { object: chatAuthorizationObject, @@ -876,20 +865,16 @@ const AgentChatPage: FC = () => { ...checkAuthorization({ checks: chatAuthorizationChecks }), enabled: Object.keys(chatAuthorizationChecks).length > 0, }); - const canUpdateOtherUserChat = Boolean( - chatAuthorizationQuery.data?.canUpdateChat, - ); - const canUpdateOtherUserChatLoading = - shouldCheckCanUpdateOtherUserChat && chatAuthorizationQuery.isLoading; const canShareChat = isRootChat && Boolean(chatAuthorizationQuery.data?.canShareChat); - const chatOwner = - isViewerNotOwner && chatRecord?.owner_username - ? { - username: chatRecord.owner_username, - ...(chatRecord.owner_name ? { name: chatRecord.owner_name } : {}), - } - : undefined; + const chatOwner = isViewerNotOwner + ? { + ...(chatRecord?.owner_username + ? { username: chatRecord.owner_username } + : {}), + ...(chatRecord?.owner_name ? { name: chatRecord.owner_name } : {}), + } + : undefined; const planModeEnabled = chatRecord?.plan_mode === "plan"; // Initialize MCP selection from chat record or defaults. @@ -1144,11 +1129,7 @@ const AgentChatPage: FC = () => { const isChatSettingsPending = isUpdateChatPlanModePending || isUpdateChatWorkspacePending; const isInputDisabled = - !hasModelOptions || - isArchived || - isChatSettingsPending || - (isViewerNotOwner && - (canUpdateOtherUserChatLoading || !canUpdateOtherUserChat)); + !hasModelOptions || isArchived || isChatSettingsPending || isViewerNotOwner; const selectedWorkspaceId = chatQuery.data?.workspace_id ?? null; const isWorkspaceLoading = @@ -1598,8 +1579,6 @@ const AgentChatPage: FC = () => { persistedError={persistedError} isArchived={isArchived} chatOwner={chatOwner} - canUpdateOtherUserChat={canUpdateOtherUserChat} - canUpdateOtherUserChatLoading={canUpdateOtherUserChatLoading} canShareChat={canShareChat} workspace={workspace} workspaceAgent={workspaceAgent} diff --git a/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx b/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx index 441984b059..baed2e4d8e 100644 --- a/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx +++ b/site/src/pages/AgentsPage/AgentChatPageView.stories.tsx @@ -153,8 +153,6 @@ const StoryAgentChatPageView: FC = ({ editing, ...overrides }) => { modelSelectorPlaceholder: "Select a model", hasModelOptions: true, compressionThreshold: undefined as number | undefined, - canUpdateOtherUserChat: false, - canUpdateOtherUserChatLoading: false, isInputDisabled: false, isSubmissionPending: false, isInterruptPending: false, @@ -231,7 +229,7 @@ export const Default: Story = { play: async ({ canvasElement }) => { const canvas = within(canvasElement); expect( - canvas.queryByText(/^This is not your chat/), + canvas.queryByText(/^This chat is owned by/), ).not.toBeInTheDocument(); }, }; @@ -241,38 +239,20 @@ export const Archived: Story = { render: () => , }; -export const AdminViewingOtherUserChat: Story = { +export const OtherUserChatReadOnly: Story = { render: () => ( - ), - play: async ({ canvasElement }) => { - const canvas = within(canvasElement); - const banner = canvas.getByText( - "This is not your chat. Prompting here will use Other User's identity.", - ); - expect(banner).toBeVisible(); - expect(banner).toHaveAttribute("role", "status"); - }, -}; - -export const OtherUserChatOwnerPending: Story = { - render: () => ( - ), play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect( - canvas.queryByText(/^This is not your chat/), - ).not.toBeInTheDocument(); - expect(canvas.queryByText(/other-user-id/)).not.toBeInTheDocument(); + const banner = canvas.getByText( + "This chat is owned by Other User. It is read-only.", + ); + expect(banner).toBeVisible(); + expect(banner).toHaveAttribute("role", "status"); expect(canvas.getByLabelText("Chat message")).toHaveAttribute( "aria-disabled", "true", @@ -280,32 +260,20 @@ export const OtherUserChatOwnerPending: Story = { }, }; -export const ReadOnlyOtherUserChatOwner: Story = { - render: () => ( - - ), - play: async ({ canvasElement }) => { - const canvas = within(canvasElement); - const banner = canvas.getByText( - "This chat is owned by @OtherUser. You have read-only access.", - ); - expect(banner).toBeVisible(); - expect(banner).toHaveAttribute("role", "status"); - }, -}; - -export const ReadOnlyOtherUserChatOwnerPending: Story = { +export const OtherUserChatUsernameFallback: Story = { render: () => ( ), play: async ({ canvasElement }) => { const canvas = within(canvasElement); - expect(canvas.queryByText(/^This chat is owned/)).not.toBeInTheDocument(); - expect(canvas.queryByText(/other-user-id/)).not.toBeInTheDocument(); + const banner = canvas.getByText( + "This chat is owned by @OtherUser. It is read-only.", + ); + expect(banner).toBeVisible(); + expect(banner).toHaveAttribute("role", "status"); expect(canvas.getByLabelText("Chat message")).toHaveAttribute( "aria-disabled", "true", @@ -313,7 +281,23 @@ export const ReadOnlyOtherUserChatOwnerPending: Story = { }, }; -/** Archived chats stay read-only without the identity warning banner. */ +export const OtherUserChatOwnerFallback: Story = { + render: () => , + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const banner = canvas.getByText( + "This chat is owned by another user. It is read-only.", + ); + expect(banner).toBeVisible(); + expect(banner).toHaveAttribute("role", "status"); + expect(canvas.getByLabelText("Chat message")).toHaveAttribute( + "aria-disabled", + "true", + ); + }, +}; + +/** Archived chats stay read-only without the owner banner. */ export const ArchivedOtherUserChat: Story = { render: () => ( { const canvas = within(canvasElement); expect( - canvas.queryByText(/^This is not your chat/), + canvas.queryByText(/^This chat is owned by/), ).not.toBeInTheDocument(); expect( canvas.getByText("This agent has been archived and is read-only."), @@ -775,18 +759,25 @@ export const LoadingSidebarCollapsed: Story = { // Helpers for seeding stores with messages // --------------------------------------------------------------------------- -const buildMessage = ( +const buildMessageWithContent = ( id: number, role: TypesGen.ChatMessageRole, - text: string, + content: TypesGen.ChatMessagePart[], ): TypesGen.ChatMessage => ({ id, chat_id: AGENT_ID, created_at: new Date(Date.now() - (10 - id) * 60_000).toISOString(), role, - content: [{ type: "text", text }], + content, }); +const buildMessage = ( + id: number, + role: TypesGen.ChatMessageRole, + text: string, +): TypesGen.ChatMessage => + buildMessageWithContent(id, role, [{ type: "text", text }]); + const buildStoreWithMessages = ( msgs: TypesGen.ChatMessage[], status: TypesGen.ChatStatus = "completed", @@ -797,6 +788,52 @@ const buildStoreWithMessages = ( return store; }; +const otherUserActionMessages: TypesGen.ChatMessage[] = [ + buildMessage(1, "user", "Please review this plan."), + buildMessageWithContent(2, "assistant", [ + { type: "text", text: "I prepared a plan." }, + { + type: "tool-call", + tool_call_id: "other-user-plan", + tool_name: "propose_plan", + args: { path: "/home/coder/PLAN.md" }, + }, + { + type: "tool-result", + tool_call_id: "other-user-plan", + tool_name: "propose_plan", + result: { + file_id: "other-user-plan-file", + content: "# Plan\n\n1. Keep this chat read-only.", + }, + }, + ]), +]; + +export const OtherUserChatHidesInlineActions: Story = { + render: () => ( + + ), + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + expect( + canvas.getByText("This chat is owned by Other User. It is read-only."), + ).toBeVisible(); + expect(await canvas.findByText("Please review this plan.")).toBeVisible(); + expect( + canvas.queryByRole("button", { name: "Edit message" }), + ).not.toBeInTheDocument(); + expect( + canvas.queryByRole("button", { name: "Implement plan" }), + ).not.toBeInTheDocument(); + }, +}; + // --------------------------------------------------------------------------- // Editing flow stories // --------------------------------------------------------------------------- diff --git a/site/src/pages/AgentsPage/AgentChatPageView.tsx b/site/src/pages/AgentsPage/AgentChatPageView.tsx index a99e3ece9e..b71f4bf0c4 100644 --- a/site/src/pages/AgentsPage/AgentChatPageView.tsx +++ b/site/src/pages/AgentsPage/AgentChatPageView.tsx @@ -51,6 +51,11 @@ import type { ChatDetailError } from "./utils/usageLimitMessage"; type ChatStoreHandle = ReturnType["store"]; +type ChatOwnerInfo = { + name?: string; + username?: string; +}; + // Re-use the inner presentational components directly. They are interface EditingState { @@ -93,9 +98,7 @@ interface AgentChatPageViewProps { parentChat: TypesGen.Chat | undefined; persistedError: ChatDetailError | undefined; isArchived: boolean; - chatOwner: Pick | undefined; - canUpdateOtherUserChat: boolean; - canUpdateOtherUserChatLoading: boolean; + chatOwner: ChatOwnerInfo | undefined; canShareChat: boolean; workspaceAgent?: TypesGen.WorkspaceAgent; workspace?: TypesGen.Workspace; @@ -201,8 +204,6 @@ export const AgentChatPageView: FC = ({ persistedError, isArchived, chatOwner, - canUpdateOtherUserChat, - canUpdateOtherUserChatLoading, canShareChat, workspaceAgent, workspace, @@ -422,16 +423,14 @@ export const AgentChatPageView: FC = ({ editing.editingMessageId !== null || editing.editingQueuedMessageID !== null; - const chatOwnerUsername = chatOwner?.username.trim(); + const chatOwnerUsername = chatOwner?.username?.trim(); const chatOwnerLabel = chatOwner?.name?.trim() || - (chatOwnerUsername ? `@${chatOwnerUsername}` : undefined); - const chatOwnerWarning = - isArchived || canUpdateOtherUserChatLoading || chatOwnerLabel === undefined - ? undefined - : canUpdateOtherUserChat - ? `This is not your chat. Prompting here will use ${chatOwnerLabel}'s identity.` - : `This chat is owned by ${chatOwnerLabel}. You have read-only access.`; + (chatOwnerUsername ? `@${chatOwnerUsername}` : "another user"); + const isOtherUserReadOnly = !isArchived && chatOwner !== undefined; + const chatOwnerWarning = isOtherUserReadOnly + ? `This chat is owned by ${chatOwnerLabel}. It is read-only.` + : undefined; const titleElement = ( @@ -535,12 +534,22 @@ export const AgentChatPageView: FC<AgentChatPageViewProps> = ({ chatID={agentId} store={store} persistedError={persistedError} - onEditUserMessage={editing.handleEditUserMessage} + onEditUserMessage={ + isOtherUserReadOnly + ? undefined + : editing.handleEditUserMessage + } editingMessageId={editing.editingMessageId} urlTransform={urlTransform} mcpServers={mcpServers} - onImplementPlan={onImplementPlan} - onSendAskUserQuestionResponse={canSendAskUserQuestionResponse} + onImplementPlan={ + isOtherUserReadOnly ? undefined : onImplementPlan + } + onSendAskUserQuestionResponse={ + isOtherUserReadOnly + ? undefined + : canSendAskUserQuestionResponse + } /> </div> </ChatScrollContainer>