From 1d97c9b047c602d08bbb0958bcc6890905ad4670 Mon Sep 17 00:00:00 2001 From: Roman Lutz Date: Sat, 10 Oct 2026 23:59:18 -0700 Subject: [PATCH] Fix conversation drawer selection races Apply the historical drawer default once per attack so late send refreshes cannot override the operator's panel choice. Synchronize selection on expanded state and verify destination routing and current rows. Cover closing drawers and delayed historical metadata on desktop and narrow screens without weakening recovered-image assertions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- frontend/README.md | 12 ++ frontend/e2e/chat-recovery.spec.ts | 158 +++++++++++++++++- .../src/components/Chat/ChatWindow.test.tsx | 34 ++++ frontend/src/components/Chat/ChatWindow.tsx | 18 +- 4 files changed, 207 insertions(+), 15 deletions(-) diff --git a/frontend/README.md b/frontend/README.md index 4164661c99..ead30c2f00 100644 --- a/frontend/README.md +++ b/frontend/README.md @@ -13,6 +13,14 @@ Your choice is saved in this browser. System follows the operating system's light/dark preference. High-contrast mode overrides every palette and hides decorations without forgetting the selected preset. +## Conversation selection + +Opening a saved attack with related conversations automatically opens the +desktop conversation drawer. This default applies once per attack; later send +refreshes do not override your open/closed choice. Creating, copying, or repeating +conversations still opens the desktop drawer. On narrow screens the drawer opens +only when requested and closes after selecting a conversation. + ## Resuming a scenario run Failed runs offer **Resume run** on the run page and **Resume** in Scanner History. @@ -161,6 +169,10 @@ E2E flow tests run in two modes controlled by Playwright projects and an environ - **Live** (`--project live`, requires `E2E_LIVE_MODE=true`): Messages are sent to real OpenAI endpoints with `send: true`. Each target variant requires endpoint and model environment variables plus either an API key or an Azure endpoint accessible through the current Entra identity. Variants without a usable configuration are automatically skipped. Tests verify that real target responses render correctly. +Conversation-selection helpers check the toggle's `aria-expanded` state, not +drawer visibility during an animation. Use accessible row clicks and wait for +the destination URL and transcript before asserting its contents. + ```bash # Seeded integration (no credentials needed) npx playwright test --project seeded diff --git a/frontend/e2e/chat-recovery.spec.ts b/frontend/e2e/chat-recovery.spec.ts index 6f40d3b920..b31f3e96c7 100644 --- a/frontend/e2e/chat-recovery.spec.ts +++ b/frontend/e2e/chat-recovery.spec.ts @@ -13,6 +13,7 @@ import { compatibilityHeaders } from "./_compatibility"; import type { AddMessageRequest, AddMessageResponse, + AttackSummary, ConversationMessagesResponse, CreateAttackResponse, CreateConversationResponse, @@ -162,20 +163,42 @@ async function sendFromComposer(page: Page, text?: string): Promise { +async function createConversation( + request: APIRequestContext, attackId: string, message?: string, +): Promise { const response = await request.post(`/api/attacks/${attackId}/conversations`, { data: {}, headers: compatibilityHeaders(), }); expect(response.status()).toBe(201); const created: CreateConversationResponse = await response.json(); + if (message !== undefined) { + const stored = await request.post(`/api/attacks/${attackId}/messages`, { + headers: compatibilityHeaders(), + data: { + role: "user", + pieces: [{ data_type: "text", original_value: message }], + send: false, + target_conversation_id: created.conversation_id, + }, + }); + expect(stored.ok()).toBeTruthy(); + } return created.conversation_id; } async function selectConversation(page: Page, conversationId: string): Promise { - if (!await page.getByTestId("conversation-panel").isVisible()) { + const toggle = page.getByTestId("toggle-panel-btn"); + await expect(toggle).toBeEnabled(); + if (await toggle.getAttribute("aria-expanded") === "false") { await page.getByRole("button", { name: "Toggle conversations panel", exact: true }).click(); } - await page.getByRole("button", { name: `Select conversation ${conversationId}`, exact: true }).click(); + await expect(toggle).toHaveAttribute("aria-expanded", "true"); + const conversation = page.getByRole("button", { name: `Select conversation ${conversationId}`, exact: true }); + await conversation.click(); + await expect(page).toHaveURL((url: URL) => url.pathname.endsWith(`/conversations/${conversationId}`)); + if (await toggle.getAttribute("aria-expanded") === "true") { + await expect(conversation).toHaveAttribute("aria-current", "true"); + } } async function installDeferredFileReads(page: Page): Promise { @@ -220,6 +243,135 @@ test.describe("Chat processing recovery @seeded", () => { await page.getByTitle("Chat", { exact: true }).click(); }); + test("keeps the drawer closed when a delayed send refresh discovers another conversation", async ({ + page, request, localTarget, + }) => { + let releaseRefresh: () => void = () => {}; + const refreshGate = new Promise((resolve) => { releaseRefresh = resolve; }); + let refreshStarted = false; + await page.route(/\/api\/attacks\/[^/?]+$/, async (route: Route) => { + if (route.request().method() === "GET") { + refreshStarted = true; + await refreshGate; + } + await route.continue(); + }); + try { + const first = await sendFromComposer(page, "Original conversation history"); + const attackId = first.attack.attack_result_id; + await expect.poll(() => refreshStarted).toBe(true); + const otherId = await createConversation(request, attackId, "Selected conversation history"); + const toggle = page.getByTestId("toggle-panel-btn"); + await expect(toggle).toHaveAttribute("aria-expanded", "false"); + await expect(page.getByRole("button", { name: "Export conversation", exact: true })).toBeDisabled(); + const refreshed = page.waitForResponse((response) => response.request().method() === "GET" + && new URL(response.url()).pathname === `/api/attacks/${attackId}`); + releaseRefresh(); + const response = await refreshed; + expect(response.ok()).toBeTruthy(); + const attack: AttackSummary = await response.json(); + expect(attack.related_conversations).toEqual(expect.arrayContaining([ + expect.objectContaining({ conversation_id: otherId, conversation_type: "pruned" }), + ])); + await expect(page.getByRole("button", { name: "Export conversation", exact: true })).toBeEnabled(); + await expect(toggle).toHaveAttribute("aria-expanded", "false"); + + await selectConversation(page, otherId); + const transcript = page.getByTestId("message-list"); + await expect(transcript.getByText("Selected conversation history", { exact: true })).toBeVisible(); + await expect(transcript.getByText("Original conversation history", { exact: true })).toHaveCount(0); + expect(localTarget.requestBodies).toHaveLength(1); + } finally { + releaseRefresh(); + } + }); + + for (const viewport of [ + { name: "desktop", width: 1280, height: 720 }, + { name: "narrow", width: 390, height: 844 }, + ]) { + test.describe(`${viewport.name} conversation selection`, () => { + test.use({ viewport: { width: viewport.width, height: viewport.height } }); + + test("preserves historical drawer defaults after delayed metadata", async ({ + page, request, localTarget, + }) => { + const first = await sendFromComposer(page, "Original conversation history"); + await expect(page.getByRole("button", { name: "Export conversation", exact: true })).toBeEnabled(); + const attackId = first.attack.attack_result_id; + const otherId = await createConversation(request, attackId, "Selected conversation history"); + let releaseMetadata: () => void = () => {}; + const metadataGate = new Promise((resolve) => { releaseMetadata = resolve; }); + let metadataStarted = false; + await page.route((url: URL) => url.pathname === `/api/attacks/${attackId}`, async (route: Route) => { + if (route.request().method() === "GET") { + metadataStarted = true; + await metadataGate; + } + await route.continue(); + }); + try { + await page.goto(`/attacks/${attackId}`); + await expect.poll(() => metadataStarted).toBe(true); + await expect(page.getByText("Loading conversation...", { exact: true })).toBeVisible(); + const toggle = page.getByTestId("toggle-panel-btn"); + await expect(toggle).toHaveAttribute("aria-expanded", "false"); + await expect(toggle).toBeDisabled(); + releaseMetadata(); + + const transcript = page.getByTestId("message-list"); + await expect(transcript.getByText("Original conversation history", { exact: true })).toBeVisible(); + await expect(toggle).toHaveAttribute("aria-expanded", viewport.name === "narrow" ? "false" : "true"); + if (viewport.name === "narrow") { + await expect(page.getByRole("dialog", { name: "Attack Conversations", exact: true })).toHaveCount(0); + } else { + await expect(page.getByRole("complementary", { name: "Attack Conversations", exact: true })).toBeVisible(); + await expect(page.getByRole("button", { + name: `Select conversation ${first.attack.conversation_id}`, exact: true, + })).toHaveAttribute("aria-current", "true"); + } + + await selectConversation(page, otherId); + await expect(transcript.getByText("Selected conversation history", { exact: true })).toBeVisible(); + await expect(transcript.getByText("Original conversation history", { exact: true })).toHaveCount(0); + expect(localTarget.requestBodies).toHaveLength(1); + } finally { + releaseMetadata(); + } + }); + + test("selects a conversation while the drawer is closing", async ({ + page, request, localTarget, + }) => { + const first = await sendFromComposer(page, "Original conversation history"); + await expect(page.getByRole("button", { name: "Export conversation", exact: true })).toBeEnabled(); + const otherId = await createConversation( + request, first.attack.attack_result_id, "Selected conversation history", + ); + const toggle = page.getByTestId("toggle-panel-btn"); + await page.getByRole("button", { name: "Toggle conversations panel", exact: true }).click(); + await expect(page.getByRole(viewport.name === "narrow" ? "dialog" : "complementary", { + name: "Attack Conversations", exact: true, + })).toBeVisible(); + await expect(page.getByRole("button", { name: `Select conversation ${otherId}`, exact: true })).toBeVisible(); + await page.getByRole("button", { name: "Close panel", exact: true }).click(); + await expect(toggle).toHaveAttribute("aria-expanded", "false"); + + await selectConversation(page, otherId); + + const transcript = page.getByTestId("message-list"); + await expect(transcript.getByText("Selected conversation history", { exact: true })).toBeVisible(); + await expect(transcript.getByText("Original conversation history", { exact: true })).toHaveCount(0); + await expect(toggle).toHaveAttribute("aria-expanded", viewport.name === "narrow" ? "false" : "true"); + if (viewport.name === "narrow") { + await expect(page.getByRole("dialog", { name: "Attack Conversations", exact: true })).toHaveCount(0); + await expect(page.getByRole("button", { name: "Toggle conversations panel", exact: true })).toBeFocused(); + } + expect(localTarget.requestBodies).toHaveLength(1); + }); + }); + } + test("repeats and nests only the selected conversation through the real backend", async ({ page, request, localTarget }, testInfo) => { let submissions = 0; page.on("request", (outgoing: Request) => { if (isMessagePost(outgoing)) submissions += 1; }); diff --git a/frontend/src/components/Chat/ChatWindow.test.tsx b/frontend/src/components/Chat/ChatWindow.test.tsx index c9634f164b..61f9effd6b 100644 --- a/frontend/src/components/Chat/ChatWindow.test.tsx +++ b/frontend/src/components/Chat/ChatWindow.test.tsx @@ -5141,6 +5141,40 @@ describe("ChatWindow Integration", () => { expect(screen.queryByTestId("conversation-panel")).not.toBeInTheDocument(); }); + it.each([false, true])("should preserve the operator's panel choice after a late conversation update, narrow %s", async (isNarrowScreen: boolean) => { + const user = userEvent.setup(); + mockMatchMedia(isNarrowScreen); + const props = { + ...defaultProps, + attackResultId: "ar-late-conversations", + conversationId: "conv-main", + activeConversationId: "conv-main", + relatedConversationCount: 0, + }; + const rendered = render( + + ); + const toggleButton = screen.getByRole("button", { name: "Toggle conversations panel" }); + expect(toggleButton).toHaveAttribute("aria-expanded", "false"); + + await user.click(toggleButton); + await user.click(await screen.findByRole("button", { name: "Close panel" })); + await waitFor(() => { + expect(screen.queryByRole(isNarrowScreen ? "dialog" : "complementary", { + name: "Attack Conversations", + })).not.toBeInTheDocument(); + }); + + rendered.rerender( + + ); + + expect(toggleButton).toHaveAttribute("aria-expanded", "false"); + expect(screen.queryByRole(isNarrowScreen ? "dialog" : "complementary", { + name: "Attack Conversations", + })).not.toBeInTheDocument(); + }); + it("should keep the mobile drawer closed until requested and restore focus after Escape", async () => { const user = userEvent.setup(); mockMatchMedia(true); diff --git a/frontend/src/components/Chat/ChatWindow.tsx b/frontend/src/components/Chat/ChatWindow.tsx index 439e28f23b..4e6a2b2329 100644 --- a/frontend/src/components/Chat/ChatWindow.tsx +++ b/frontend/src/components/Chat/ChatWindow.tsx @@ -452,18 +452,12 @@ export default function ChatWindow({ [activePieceConversions, converters.pipelines, converters.editRevision], ) - // Auto-open conversation sidebar when loading a historical attack with multiple - // conversations. Uses the "adjust state during render" pattern to avoid - // react-hooks/set-state-in-effect. - const [autoOpenedForAttack, setAutoOpenedForAttack] = useState(null) - if ( - attackResultId - && attackResultId !== autoOpenedForAttack - && relatedConversationCount - && relatedConversationCount > 0 - ) { - setAutoOpenedForAttack(attackResultId) - if (!isNarrowScreen) { + // Apply the historical attack's drawer default once. Late send refreshes + // must not override the operator's open/closed choice. + const [panelInitializedForAttack, setPanelInitializedForAttack] = useState(null) + if (attackResultId && attackResultId !== panelInitializedForAttack) { + setPanelInitializedForAttack(attackResultId) + if (!isNarrowScreen && relatedConversationCount && relatedConversationCount > 0) { setIsPanelOpen(true) } }