From a74409af3c28bef4edc708f07835d28a89cc2348 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Flor=C3=AAncio?= Date: Tue, 19 May 2026 22:27:24 +0100 Subject: [PATCH] fix(web): cap clarification overlay height and add mock agent ask commands (#956) --- apps/backend/cmd/mock-agent/handler.go | 6 + apps/backend/cmd/mock-agent/main.go | 2 + .../quick-chat/quick-chat-content.tsx | 32 +++- .../task/chat/clarification-input-overlay.tsx | 36 ++-- .../components/task/chat/resize-handle.tsx | 19 +- apps/web/components/task/task-chat-panel.tsx | 32 +++- .../tests/chat/clarification-resize.spec.ts | 164 +++++++++++++++++ ...se-resizable-clarification-overlay.test.ts | 172 ++++++++++++++++++ .../use-resizable-clarification-overlay.ts | 85 +++++++++ 9 files changed, 513 insertions(+), 35 deletions(-) create mode 100644 apps/web/e2e/tests/chat/clarification-resize.spec.ts create mode 100644 apps/web/hooks/use-resizable-clarification-overlay.test.ts create mode 100644 apps/web/hooks/use-resizable-clarification-overlay.ts diff --git a/apps/backend/cmd/mock-agent/handler.go b/apps/backend/cmd/mock-agent/handler.go index fa523715d1..c8004f57d1 100644 --- a/apps/backend/cmd/mock-agent/handler.go +++ b/apps/backend/cmd/mock-agent/handler.go @@ -74,6 +74,12 @@ func handlePrompt(e *emitter, prompt, model string) { rest := strings.TrimPrefix(cmd, "/e2e:") scenarioName, _, _ := strings.Cut(strings.TrimSpace(rest), " ") emitPredefinedScenario(e, scenarioName) + // Friendly aliases for the two clarification e2e scenarios so the slash menu + // exposes them without forcing users to type /e2e:clarification(-multi). + case strings.EqualFold(cmd, "/ask-single"): + emitPredefinedScenario(e, "clarification") + case strings.EqualFold(cmd, "/ask-multiple"): + emitPredefinedScenario(e, "clarification-multi") case strings.EqualFold(cmd, "/crash"): emitCrash(e, model) case strings.HasPrefix(cmd, "/todo"): diff --git a/apps/backend/cmd/mock-agent/main.go b/apps/backend/cmd/mock-agent/main.go index e15dac27c1..55992687d8 100644 --- a/apps/backend/cmd/mock-agent/main.go +++ b/apps/backend/cmd/mock-agent/main.go @@ -266,6 +266,8 @@ func mockAvailableCommands() []acp.AvailableCommand { {Name: "tool:search", Description: "Emit a search tool call"}, {Name: "markdown", Description: "Render all markdown/text types"}, {Name: "sleep", Description: "Sleep without output (default 10s)", Input: hint("seconds (e.g. 30)")}, + {Name: "ask-single", Description: "Ask a single clarification question"}, + {Name: "ask-multiple", Description: "Ask multiple clarification questions"}, } } diff --git a/apps/web/components/quick-chat/quick-chat-content.tsx b/apps/web/components/quick-chat/quick-chat-content.tsx index a50dddf585..64e3ebb7c4 100644 --- a/apps/web/components/quick-chat/quick-chat-content.tsx +++ b/apps/web/components/quick-chat/quick-chat-content.tsx @@ -12,6 +12,8 @@ import { useChatPanelHandlers, } from "@/components/task/chat/chat-input-area"; import { ClarificationInputOverlay } from "@/components/task/chat/clarification-input-overlay"; +import { ResizeHandle } from "@/components/task/chat/resize-handle"; +import { useResizableClarificationOverlay } from "@/hooks/use-resizable-clarification-overlay"; type QuickChatContentProps = { sessionId: string; @@ -73,6 +75,18 @@ export const QuickChatContent = memo(function QuickChatContent({ }, [initialPrompt, taskId, handleSubmit, onInitialPromptSent]); const handleClarificationResolved = useCallback(() => setClarificationKey((k) => k + 1), []); + const { + height: clarificationHeight, + containerRef: clarificationContainerRef, + resetHeight: clarificationResetHeight, + resizeHandleProps: clarificationResizeProps, + } = useResizableClarificationOverlay(); + + // Reset the dragged height when the overlay closes so a fresh + // clarification starts auto-sized instead of inheriting a stale value. + useEffect(() => { + if (!pendingClarification) clarificationResetHeight(); + }, [pendingClarification, clarificationResetHeight]); return (
@@ -93,11 +107,19 @@ export const QuickChatContent = memo(function QuickChatContent({ />
{pendingClarification && ( -
- +
+ +
+ +
)} diff --git a/apps/web/components/task/chat/clarification-input-overlay.tsx b/apps/web/components/task/chat/clarification-input-overlay.tsx index 20dfc8036a..34d63af7fc 100644 --- a/apps/web/components/task/chat/clarification-input-overlay.tsx +++ b/apps/web/components/task/chat/clarification-input-overlay.tsx @@ -90,22 +90,23 @@ function ClarificationCard(props: CardProps) { data-testid="clarification-question-card" data-question-id={meta.questionId} data-question-index={String(index)} - className="px-4 pt-3 pb-4" + className="px-4 pt-1 pb-4" > -
- - {total > 1 && ( - - Question {index + 1} of {total} - - )} - {metadata.question.title && ( - - {total > 1 ? "· " : ""} - {metadata.question.title} - - )} -
+ {(total > 1 || metadata.question.title) && ( +
+ {total > 1 && ( + + Question {index + 1} of {total} + + )} + {metadata.question.title && ( + + {total > 1 ? "· " : ""} + {metadata.question.title} + + )} +
+ )}
{question.prompt} @@ -453,8 +454,9 @@ export function ClarificationInputOverlay({ return (
-
-
+
+
+ {total > 1 && (
diff --git a/apps/web/components/task/task-chat-panel.tsx b/apps/web/components/task/task-chat-panel.tsx index 373e013a53..b29bd8a661 100644 --- a/apps/web/components/task/task-chat-panel.tsx +++ b/apps/web/components/task/task-chat-panel.tsx @@ -10,6 +10,8 @@ import { useIsTaskArchived } from "./task-archived-context"; import { useChatPanelState } from "./chat/use-chat-panel-state"; import { ChatInputArea, useSubmitHandler, useChatPanelHandlers } from "./chat/chat-input-area"; import { ClarificationInputOverlay } from "./chat/clarification-input-overlay"; +import { ResizeHandle } from "./chat/resize-handle"; +import { useResizableClarificationOverlay } from "@/hooks/use-resizable-clarification-overlay"; import { PanelSearchBar } from "@/components/search/panel-search-bar"; import { SessionSearchHits } from "@/components/task/chat/session-search-hits"; import { usePanelSearch } from "@/hooks/use-panel-search"; @@ -157,6 +159,18 @@ export const TaskChatPanel = memo(function TaskChatPanel({ } = panelState; const { handleCancelTurn } = useChatPanelHandlers(resolvedSessionId, clearQueue, chatInputRef); const { clarificationKey, handleClarificationResolved } = useClarificationKey(agentMessageCount); + const { + height: clarificationHeight, + containerRef: clarificationContainerRef, + resetHeight: clarificationResetHeight, + resizeHandleProps: clarificationResizeProps, + } = useResizableClarificationOverlay(); + + // Reset the dragged height when the overlay closes so a fresh + // clarification starts auto-sized instead of inheriting a stale value. + useEffect(() => { + if (!pendingClarification) clarificationResetHeight(); + }, [pendingClarification, clarificationResetHeight]); const panelRef = useRef(null); const { loadMore } = useLazyLoadMessages(resolvedSessionId); @@ -216,11 +230,19 @@ export const TaskChatPanel = memo(function TaskChatPanel({ {pendingClarification && !isArchived && ( -
- +
+ +
+ +
)} diff --git a/apps/web/e2e/tests/chat/clarification-resize.spec.ts b/apps/web/e2e/tests/chat/clarification-resize.spec.ts new file mode 100644 index 0000000000..dda5ea422d --- /dev/null +++ b/apps/web/e2e/tests/chat/clarification-resize.spec.ts @@ -0,0 +1,164 @@ +import { type Page } from "@playwright/test"; +import { test, expect } from "../../fixtures/test-base"; +import type { SeedData } from "../../fixtures/test-base"; +import type { ApiClient } from "../../helpers/api-client"; +import { SessionPage } from "../../pages/session-page"; + +/** + * Helper to create a regular task, navigate to it, and wait for idle. + * Mirrors clarification.spec.ts's `seedClarificationTask`, but uses a non- + * blocking scenario so we reach the idle input and can drive the slash + * commands `/ask-single` and `/ask-multiple` from inside the session. + */ +async function seedTaskAndWaitForIdle( + testPage: Page, + apiClient: ApiClient, + seedData: SeedData, + title: string, +): Promise { + const task = await apiClient.createTaskWithAgent( + seedData.workspaceId, + title, + seedData.agentProfileId, + { + description: "/e2e:simple-message", + workflow_id: seedData.workflowId, + workflow_step_id: seedData.startStepId, + repository_ids: [seedData.repositoryId], + }, + ); + + if (!task.session_id) throw new Error("createTaskWithAgent did not return a session_id"); + + await testPage.goto(`/t/${task.id}`); + + const session = new SessionPage(testPage); + await session.waitForLoad(); + await session.waitForChatIdle(); + + return session; +} + +test.describe("Mock agent clarification slash commands", () => { + test.describe.configure({ retries: 1 }); + + // Smoke test that the /ask-single alias routes to the clarification scenario. + // The underlying clarification behaviour (option click, multi-question carousel, + // submit, etc.) is exhaustively covered by clarification.spec.ts — this only + // proves the alias is wired up. + test("/ask-single alias triggers the clarification overlay", async ({ + testPage, + apiClient, + seedData, + }) => { + const session = await seedTaskAndWaitForIdle(testPage, apiClient, seedData, "Ask Single Alias"); + + await session.sendMessage("/ask-single"); + + await expect(session.clarificationOverlay()).toBeVisible({ timeout: 30_000 }); + await session.clarificationOption("PostgreSQL").click(); + await expect(session.idleInput()).toBeVisible({ timeout: 30_000 }); + }); + + test("/ask-multiple alias triggers the multi-question carousel", async ({ + testPage, + apiClient, + seedData, + }) => { + const session = await seedTaskAndWaitForIdle( + testPage, + apiClient, + seedData, + "Ask Multiple Alias", + ); + + await session.sendMessage("/ask-multiple"); + + await expect(session.clarificationOverlay()).toBeVisible({ timeout: 30_000 }); + await expect(session.clarificationSteps()).toHaveCount(3); + await expect(session.clarificationOverlay()).toContainText("Which database"); + + await session.clarificationOption("PostgreSQL").click(); + await session.clarificationOption("Go").click(); + await session.clarificationOption("Docker").click(); + + await session.clarificationSubmit().click(); + await expect(session.clarificationOverlay()).not.toBeVisible({ timeout: 30_000 }); + await expect(session.idleInput()).toBeVisible({ timeout: 30_000 }); + }); +}); + +test.describe("Clarification overlay resizable layout", () => { + test("starts content-sized, drag grows the overlay, double-click resets", async ({ + testPage, + apiClient, + seedData, + }) => { + // Tall viewport so the content-sized overlay has plenty of room to grow + // without bumping into the 50vh safety cap on the first drag. + await testPage.setViewportSize({ width: 1280, height: 1000 }); + + const session = await seedTaskAndWaitForIdle( + testPage, + apiClient, + seedData, + "Clarification Resize", + ); + + await session.sendMessage("/ask-multiple"); + await expect(session.clarificationOverlay()).toBeVisible({ timeout: 30_000 }); + + const container = testPage.getByTestId("clarification-overlay-container"); + await expect(container).toBeVisible(); + + const initial = await container.evaluate((el) => { + const computed = window.getComputedStyle(el); + return { + overflowY: computed.overflowY, + height: el.getBoundingClientRect().height, + // Inline style is what disambiguates "auto-sized" from "user-dragged". + inlineHeight: (el as HTMLElement).style.height, + }; + }); + + expect(initial.overflowY).toBe("auto"); + // Default state: no inline height → container sizes to its content. + expect(initial.inlineHeight).toBe(""); + // Sanity check: content-sized overlay is at least tall enough for the + // question card but well under the 50vh cap. + expect(initial.height).toBeGreaterThan(200); + expect(initial.height).toBeLessThan(1000 * 0.5); + + const handle = container.locator("xpath=..").locator("button[aria-label='Resize']"); + await expect(handle).toBeVisible(); + + // Drag the handle upward by 120px → overlay should grow by ~120px. + const handleBox = await handle.boundingBox(); + expect(handleBox).not.toBeNull(); + const dragDistance = 120; + const startX = handleBox!.x + handleBox!.width / 2; + const startY = handleBox!.y + handleBox!.height / 2; + await testPage.mouse.move(startX, startY); + await testPage.mouse.down(); + await testPage.mouse.move(startX, startY - dragDistance, { steps: 10 }); + await testPage.mouse.up(); + + const afterDrag = await container.evaluate((el) => ({ + height: el.getBoundingClientRect().height, + inlineHeight: (el as HTMLElement).style.height, + })); + // Drag flipped the container to an explicit pixel height. + expect(afterDrag.inlineHeight).toMatch(/^\d+(\.\d+)?px$/); + // Allow ±10px tolerance for rounding / mouse event coalescing. + expect(afterDrag.height).toBeGreaterThan(initial.height + dragDistance - 10); + + // Double-click the handle → back to auto-sized. + await handle.dblclick(); + const afterReset = await container.evaluate((el) => ({ + height: el.getBoundingClientRect().height, + inlineHeight: (el as HTMLElement).style.height, + })); + expect(afterReset.inlineHeight).toBe(""); + expect(Math.abs(afterReset.height - initial.height)).toBeLessThanOrEqual(2); + }); +}); diff --git a/apps/web/hooks/use-resizable-clarification-overlay.test.ts b/apps/web/hooks/use-resizable-clarification-overlay.test.ts new file mode 100644 index 0000000000..84df628b92 --- /dev/null +++ b/apps/web/hooks/use-resizable-clarification-overlay.test.ts @@ -0,0 +1,172 @@ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { renderHook, act } from "@testing-library/react"; +import { useResizableClarificationOverlay } from "./use-resizable-clarification-overlay"; + +const MIN_HEIGHT = 120; + +function setInnerHeight(h: number) { + Object.defineProperty(window, "innerHeight", { configurable: true, value: h, writable: true }); +} + +// Stub getBoundingClientRect on the (detached) container ref so the hook's +// "measure current rendered height on drag start" branch has a value to read. +function attachContainerWithHeight(ref: React.RefObject, height: number) { + const el = document.createElement("div"); + el.getBoundingClientRect = () => ({ + height, + width: 800, + x: 0, + y: 0, + top: 0, + left: 0, + right: 800, + bottom: height, + toJSON: () => ({}), + }); + (ref as { current: HTMLDivElement }).current = el; +} + +function dispatchMouseMove(clientY: number) { + document.dispatchEvent(new MouseEvent("mousemove", { clientY, bubbles: true })); +} + +function dispatchMouseUp() { + document.dispatchEvent(new MouseEvent("mouseup", { bubbles: true })); +} + +// Fake MouseEvent for handleMouseDown — only clientY and preventDefault are read. +function fakeReactMouseEvent(clientY: number) { + return { clientY, preventDefault: () => {} } as unknown as React.MouseEvent; +} + +describe("useResizableClarificationOverlay", () => { + beforeEach(() => { + setInnerHeight(1000); + document.body.style.cursor = ""; + document.body.style.userSelect = ""; + }); + + afterEach(() => { + document.body.style.cursor = ""; + document.body.style.userSelect = ""; + }); + + it("starts with height=null (auto-sized) and exposes a containerRef + handle props", () => { + const { result } = renderHook(() => useResizableClarificationOverlay()); + expect(result.current.height).toBeNull(); + expect(result.current.containerRef.current).toBeNull(); + expect(typeof result.current.resizeHandleProps.onMouseDown).toBe("function"); + expect(typeof result.current.resizeHandleProps.onDoubleClick).toBe("function"); + }); + + it("drag UP from a measured 300px grows height proportionally", () => { + const { result } = renderHook(() => useResizableClarificationOverlay()); + attachContainerWithHeight(result.current.containerRef, 300); + + // Mouse down at y=500, drag up to y=400 (delta=100), expect height ≈ 400. + act(() => result.current.resizeHandleProps.onMouseDown(fakeReactMouseEvent(500))); + act(() => dispatchMouseMove(400)); + + expect(result.current.height).toBe(400); + }); + + it("clamps below MIN_HEIGHT when dragging down past it", () => { + const { result } = renderHook(() => useResizableClarificationOverlay()); + attachContainerWithHeight(result.current.containerRef, 200); + + // Drag down (clientY grows) 500px → would target -300, must clamp to MIN_HEIGHT. + act(() => result.current.resizeHandleProps.onMouseDown(fakeReactMouseEvent(200))); + act(() => dispatchMouseMove(700)); + + expect(result.current.height).toBe(MIN_HEIGHT); + }); + + it("clamps above 50% of viewport when dragging up past it", () => { + setInnerHeight(800); // 50% = 400 cap + const { result } = renderHook(() => useResizableClarificationOverlay()); + attachContainerWithHeight(result.current.containerRef, 300); + + act(() => result.current.resizeHandleProps.onMouseDown(fakeReactMouseEvent(500))); + act(() => dispatchMouseMove(0)); // delta=500 → 800 requested, must clamp to 400 + + expect(result.current.height).toBe(400); + }); + + it("does not update height after mouseup", () => { + const { result } = renderHook(() => useResizableClarificationOverlay()); + attachContainerWithHeight(result.current.containerRef, 250); + + act(() => result.current.resizeHandleProps.onMouseDown(fakeReactMouseEvent(500))); + act(() => dispatchMouseMove(450)); // delta=50 → 300 + expect(result.current.height).toBe(300); + + act(() => dispatchMouseUp()); + act(() => dispatchMouseMove(200)); // would target 550 if still dragging + + expect(result.current.height).toBe(300); + }); + + it("sets document.body cursor/userSelect during drag and restores on mouseup", () => { + const { result } = renderHook(() => useResizableClarificationOverlay()); + attachContainerWithHeight(result.current.containerRef, 200); + + act(() => result.current.resizeHandleProps.onMouseDown(fakeReactMouseEvent(400))); + expect(document.body.style.cursor).toBe("ns-resize"); + expect(document.body.style.userSelect).toBe("none"); + + act(() => dispatchMouseUp()); + expect(document.body.style.cursor).toBe(""); + expect(document.body.style.userSelect).toBe(""); + }); + + it("falls back to MIN_HEIGHT as the drag origin when the container ref is empty", () => { + const { result } = renderHook(() => useResizableClarificationOverlay()); + // No containerRef attached → fallback path. + + act(() => result.current.resizeHandleProps.onMouseDown(fakeReactMouseEvent(500))); + act(() => dispatchMouseMove(400)); // delta=100 → MIN_HEIGHT + 100 = 220 + + expect(result.current.height).toBe(MIN_HEIGHT + 100); + }); + + it("resetHeight and the handle's onDoubleClick both revert to null (auto)", () => { + const { result } = renderHook(() => useResizableClarificationOverlay()); + attachContainerWithHeight(result.current.containerRef, 250); + + act(() => result.current.resizeHandleProps.onMouseDown(fakeReactMouseEvent(500))); + act(() => dispatchMouseMove(400)); + act(() => dispatchMouseUp()); + // startHeight=250 + delta=100 (500→400) = 350 + expect(result.current.height).toBe(350); + + act(() => result.current.resetHeight()); + expect(result.current.height).toBeNull(); + + // Drag again, then double-click resets back to auto-sized. + act(() => result.current.resizeHandleProps.onMouseDown(fakeReactMouseEvent(500))); + act(() => dispatchMouseMove(380)); + act(() => dispatchMouseUp()); + expect(result.current.height).not.toBeNull(); + + act(() => result.current.resizeHandleProps.onDoubleClick()); + expect(result.current.height).toBeNull(); + }); + + it("cleans up body cursor and stops listening on unmount mid-drag", () => { + const { result, unmount } = renderHook(() => useResizableClarificationOverlay()); + attachContainerWithHeight(result.current.containerRef, 250); + + act(() => result.current.resizeHandleProps.onMouseDown(fakeReactMouseEvent(500))); + expect(document.body.style.cursor).toBe("ns-resize"); + + unmount(); + + // Unmount mid-drag must restore body styles. + expect(document.body.style.cursor).toBe(""); + expect(document.body.style.userSelect).toBe(""); + + // Listeners removed: a synthetic mousemove must not throw / update anything. + expect(() => dispatchMouseMove(200)).not.toThrow(); + expect(result.current.height).toBeNull(); + }); +}); diff --git a/apps/web/hooks/use-resizable-clarification-overlay.ts b/apps/web/hooks/use-resizable-clarification-overlay.ts new file mode 100644 index 0000000000..bfb7948ab9 --- /dev/null +++ b/apps/web/hooks/use-resizable-clarification-overlay.ts @@ -0,0 +1,85 @@ +"use client"; + +import { useCallback, useEffect, useRef, useState } from "react"; + +const MIN_HEIGHT = 120; +const MAX_VIEWPORT_RATIO = 0.5; + +function viewportHeight(): number { + return typeof window === "undefined" ? 800 : window.innerHeight; +} + +function clampHeight(value: number): number { + const max = Math.round(viewportHeight() * MAX_VIEWPORT_RATIO); + return Math.min(Math.max(value, MIN_HEIGHT), max); +} + +/** + * Drives the user-resizable clarification overlay. + * + * Default behaviour: the container sizes to its content (`height === null` → + * caller omits the inline height style). Once the user drags the resize + * handle, the height switches to an explicit pixel value clamped between + * MIN_HEIGHT and 50% of the viewport. Double-clicking the handle returns to + * the auto-sized default. + * + * Callers MUST invoke `resetHeight()` when the overlay closes so a fresh + * clarification starts with auto-sized height instead of inheriting a + * dragged value. + */ +export function useResizableClarificationOverlay() { + // null = auto-size to content; number = user-driven pixel height. + const [height, setHeight] = useState(null); + const containerRef = useRef(null); + const isDragging = useRef(false); + const startY = useRef(0); + const startHeight = useRef(0); + + const handleMouseDown = useCallback((e: React.MouseEvent) => { + e.preventDefault(); + // Capture the *currently rendered* height so the drag continues from + // wherever the overlay sits — whether it's auto-sized or already pinned. + const measured = containerRef.current?.getBoundingClientRect().height ?? MIN_HEIGHT; + isDragging.current = true; + startY.current = e.clientY; + startHeight.current = measured; + document.body.style.cursor = "ns-resize"; + document.body.style.userSelect = "none"; + }, []); + + const resetHeight = useCallback(() => setHeight(null), []); + + useEffect(() => { + const handleMouseMove = (e: MouseEvent) => { + if (!isDragging.current) return; + // Dragging the handle UP grows the overlay (it expands toward the top + // of the screen), so a smaller clientY → larger height. + const delta = startY.current - e.clientY; + setHeight(clampHeight(startHeight.current + delta)); + }; + const handleMouseUp = () => { + if (!isDragging.current) return; + isDragging.current = false; + document.body.style.cursor = ""; + document.body.style.userSelect = ""; + }; + document.addEventListener("mousemove", handleMouseMove); + document.addEventListener("mouseup", handleMouseUp); + return () => { + document.removeEventListener("mousemove", handleMouseMove); + document.removeEventListener("mouseup", handleMouseUp); + if (isDragging.current) { + isDragging.current = false; + document.body.style.cursor = ""; + document.body.style.userSelect = ""; + } + }; + }, []); + + return { + height, + containerRef, + resetHeight, + resizeHandleProps: { onMouseDown: handleMouseDown, onDoubleClick: resetHeight }, + }; +}