fix(web): stop Escape from silently rejecting clarification questions (#2729)

Co-authored-by: nova28 <17953305+nova28@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Co-authored-by: Carlos Florencio <carlosmflorencio@gmail.com>
This commit is contained in:
nova28
2026-08-18 06:57:51 +00:00
committed by GitHub
parent a4c346c419
commit 02827208f6
18 changed files with 1302 additions and 308 deletions
@@ -1,8 +1,6 @@
"use client";
import { memo, useCallback, useEffect, useId, useRef, useState, type RefObject } from "react";
import { IconChevronDown, IconChevronUp, IconMessageQuestion } from "@tabler/icons-react";
import { Button } from "@kandev/ui/button";
import { memo, useCallback, useEffect, useRef, useState } from "react";
import { useSettingsData } from "@/hooks/domains/settings/use-settings-data";
import { type ChatInputContainerHandle } from "@/components/task/chat/chat-input-container";
import { MessageList } from "@/components/task/chat/message-list";
@@ -12,13 +10,9 @@ import {
useSubmitHandler,
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";
import type { Message } from "@/lib/types/http";
import { ClarificationPanelSection } from "@/components/task/chat/clarification-panel-section";
import { getSessionWorkspacePath } from "@/lib/session-workspace-path";
import { routePanelMouseDown } from "@/components/task/chat/route-panel-mouse-down";
import { useTranslation } from "react-i18next";
type QuickChatContentProps = {
sessionId: string;
@@ -28,92 +22,6 @@ type QuickChatContentProps = {
onInitialPromptSent?: () => void;
};
type QuickChatClarificationSectionProps = {
pending: boolean;
messages: readonly Message[] | null | undefined;
onResolved: () => void;
shortcutScopeRef: RefObject<HTMLElement | null>;
};
function QuickChatClarificationSection({
pending,
messages,
onResolved,
shortcutScopeRef,
}: QuickChatClarificationSectionProps) {
const { t } = useTranslation();
const [collapsed, setCollapsed] = useState(false);
const contentId = useId();
const { height, containerRef, resetHeight, resizeHandleProps } =
useResizableClarificationOverlay();
// A newly opened clarification starts expanded and auto-sized. Collapsing
// an active one leaves its form state and user-selected height intact.
useEffect(() => {
if (!pending) {
setCollapsed(false);
resetHeight();
}
}, [pending, resetHeight]);
if (!pending) return null;
const actionLabel = collapsed ? t("chat:expandClarification") : t("chat:collapseClarification");
return (
<div className="relative flex-shrink-0 border-t border-sky-400/30 bg-card">
{!collapsed && <ResizeHandle {...resizeHandleProps} />}
<div
ref={containerRef}
data-testid="clarification-overlay-container"
className={
collapsed
? "h-11"
: "flex min-h-[7.5rem] max-h-[35vh] flex-col overflow-hidden overscroll-contain"
}
style={!collapsed && height !== null ? { height } : undefined}
>
<div className="flex h-11 flex-shrink-0 items-center justify-between gap-2 pl-4">
<div className="flex min-w-0 items-center gap-2 text-sm font-medium">
<IconMessageQuestion className="h-4 w-4 flex-shrink-0 text-blue-500" />
<span className="truncate">{t("chat:clarificationNeeded")}</span>
</div>
<Button
type="button"
variant="ghost"
size="icon"
className="h-11 w-11 flex-shrink-0 cursor-pointer rounded-none"
aria-label={actionLabel}
aria-expanded={!collapsed}
aria-controls={contentId}
title={actionLabel}
data-testid="clarification-collapse-toggle"
onClick={() => setCollapsed((current) => !current)}
>
{collapsed ? (
<IconChevronUp className="h-4 w-4" />
) : (
<IconChevronDown className="h-4 w-4" />
)}
</Button>
</div>
<div
id={contentId}
data-testid="clarification-scroll-region"
className={collapsed ? "hidden" : "min-h-0 flex-1 overflow-y-auto px-1"}
>
<ClarificationInputOverlay
messages={messages}
onResolved={onResolved}
shortcutScopeRef={shortcutScopeRef}
keyboardShortcutsEnabled={!collapsed}
/>
</div>
</div>
</div>
);
}
function useQuickChatState(sessionId: string) {
const chatInputRef = useRef<ChatInputContainerHandle>(null);
@@ -190,12 +98,13 @@ export const QuickChatContent = memo(function QuickChatContent({
onOpenFile={undefined}
/>
</div>
<QuickChatClarificationSection
<ClarificationPanelSection
key={sessionId}
pending={Boolean(pendingClarification)}
messages={pendingClarificationGroup}
onResolved={handleClarificationResolved}
shortcutScopeRef={shortcutScopeRef}
maxHeightVh={35}
/>
<ChatInputArea
chatInputRef={chatInputRef}
@@ -1,5 +1,7 @@
import { cleanup, render, screen } from "@testing-library/react";
import { useState } from "react";
import { cleanup, fireEvent, render, screen } from "@testing-library/react";
import { afterEach, describe, expect, it, vi } from "vitest";
import { useClarificationEscapeGuard } from "@/hooks/use-clarification-escape-guard";
const setQuickChatInitialPrompt = vi.fn();
const handleActivateTerminal = vi.fn();
@@ -7,9 +9,12 @@ const handleNewTerminal = vi.fn();
const handleCloseTerminal = vi.fn();
const handleOpenChange = vi.fn();
const handleNewChat = vi.fn();
const escapePreventDefault = vi.fn();
const CHAT_NAME = "Chat one";
const WORKSPACE_ID = "ws-1";
const TERMINAL_TAB_ID = "terminal-1";
const FIRE_ESCAPE_TESTID = "fire-escape";
const TOGGLE_ESCAPE_GUARD_TESTID = "toggle-escape-guard";
vi.mock("@/components/state-provider", () => ({
useAppStore: (
@@ -27,7 +32,24 @@ vi.mock("@/lib/routing/client-dynamic", () => ({
vi.mock("@kandev/ui/dialog", () => ({
Dialog: ({ open, children }: { open: boolean; children: React.ReactNode }) =>
open ? <div data-testid="quick-chat-dialog">{children}</div> : null,
DialogContent: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
DialogContent: ({
children,
onEscapeKeyDown,
}: {
children: React.ReactNode;
onEscapeKeyDown?: (event: { preventDefault: () => void }) => void;
}) => (
<div>
{children}
{/* Stands in for Radix's DismissableLayer consulting onEscapeKeyDown
before deciding whether to close the dialog on a real Escape press. */}
<button
type="button"
data-testid={FIRE_ESCAPE_TESTID}
onClick={() => onEscapeKeyDown?.({ preventDefault: escapePreventDefault })}
/>
</div>
),
DialogTitle: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
}));
@@ -73,8 +95,22 @@ vi.mock("@/components/config-chat/use-config-chat", () => ({
vi.mock("./quick-chat-delete-dialog", () => ({
QuickChatDeleteDialog: () => null,
}));
function EscapeGuardProbe() {
const [guarded, setGuarded] = useState(false);
useClarificationEscapeGuard(guarded);
return (
<div data-testid="quick-chat-session-view">
<button
type="button"
data-testid={TOGGLE_ESCAPE_GUARD_TESTID}
onClick={() => setGuarded((current) => !current)}
/>
</div>
);
}
vi.mock("./quick-chat-session-view", () => ({
QuickChatSessionView: () => <div data-testid="quick-chat-session-view" />,
QuickChatSessionView: () => <EscapeGuardProbe />,
}));
vi.mock("./quick-chat-setup", () => ({
QuickChatSetup: () => <div data-testid="quick-chat-setup" />,
@@ -82,54 +118,57 @@ vi.mock("./quick-chat-setup", () => ({
vi.mock("@/components/config-chat/config-chat-setup", () => ({
ConfigChatSetup: () => <div data-testid="config-chat-setup" />,
}));
const useQuickChatModalMock = vi.fn(() => defaultQuickChatModalState);
vi.mock("./use-quick-chat-modal", () => ({
useQuickChatModal: () => ({
isOpen: true,
sessions: [{ sessionId: "chat-1", workspaceId: WORKSPACE_ID, kind: "chat", name: CHAT_NAME }],
terminalTabs: [
{
tabId: TERMINAL_TAB_ID,
workspaceId: WORKSPACE_ID,
sessionId: "pty-1",
sequence: 1,
status: "running",
},
],
activeKind: "terminal",
activeSessionId: "chat-1",
activeTerminalTabId: TERMINAL_TAB_ID,
activeTerminalTab: {
useQuickChatModal: () => useQuickChatModalMock(),
}));
const defaultQuickChatModalState = {
isOpen: true,
sessions: [{ sessionId: "chat-1", workspaceId: WORKSPACE_ID, kind: "chat", name: CHAT_NAME }],
terminalTabs: [
{
tabId: TERMINAL_TAB_ID,
workspaceId: WORKSPACE_ID,
sessionId: "pty-1",
sequence: 1,
status: "running",
},
activeSession: {
sessionId: "chat-1",
workspaceId: WORKSPACE_ID,
kind: "chat",
name: CHAT_NAME,
},
sessionToClose: null,
setupKey: 0,
activeSessionNeedsAgent: true,
pendingAgentId: null,
setActiveQuickChatSession: vi.fn(),
setSessionToClose: vi.fn(),
handleOpenChange,
handleNewChat,
handleNewTerminal,
handleActivateTerminal,
handleTerminalStateChange: vi.fn(),
handleSetupKindChange: vi.fn(),
handleSelectAgent: vi.fn(),
handleCloseTab: vi.fn(),
handleCloseTerminal,
handleConfirmClose: vi.fn(),
handleRename: vi.fn(),
}),
}));
],
activeKind: "terminal",
activeSessionId: "chat-1",
activeTerminalTabId: TERMINAL_TAB_ID,
activeTerminalTab: {
tabId: TERMINAL_TAB_ID,
workspaceId: WORKSPACE_ID,
sessionId: "pty-1",
sequence: 1,
status: "running",
},
activeSession: {
sessionId: "chat-1",
workspaceId: WORKSPACE_ID,
kind: "chat",
name: CHAT_NAME,
},
sessionToClose: null,
setupKey: 0,
activeSessionNeedsAgent: true,
pendingAgentId: null,
setActiveQuickChatSession: vi.fn(),
setSessionToClose: vi.fn(),
handleOpenChange,
handleNewChat,
handleNewTerminal,
handleActivateTerminal,
handleTerminalStateChange: vi.fn(),
handleSetupKindChange: vi.fn(),
handleSelectAgent: vi.fn(),
handleCloseTab: vi.fn(),
handleCloseTerminal,
handleConfirmClose: vi.fn(),
handleRename: vi.fn(),
};
import { QuickChatModal } from "./quick-chat-modal";
@@ -156,3 +195,47 @@ describe("QuickChatModal mixed tabs", () => {
expect(screen.queryByTestId("quick-chat-menu-terminal-terminal-1")).toBeNull();
});
});
describe("QuickChatModal — Escape with a pending clarification", () => {
function renderWithConversationTab() {
// mockReturnValue (not -Once): QuickChatModal's own escape-guard state
// lives in this component and re-renders it on every guard toggle, which
// re-invokes useQuickChatModal() more than the one time a *Once value
// would cover.
useQuickChatModalMock.mockReturnValue({
...defaultQuickChatModalState,
activeKind: "conversation",
activeSessionNeedsAgent: false,
});
render(<QuickChatModal workspaceId={WORKSPACE_ID} />);
}
it("lets Escape close the modal when no clarification is guarding it", () => {
renderWithConversationTab();
fireEvent.click(screen.getByTestId(FIRE_ESCAPE_TESTID));
expect(escapePreventDefault).not.toHaveBeenCalled();
});
it("swallows the first Escape while a clarification is pending and expanded, keeping the modal open", () => {
renderWithConversationTab();
fireEvent.click(screen.getByTestId(TOGGLE_ESCAPE_GUARD_TESTID));
fireEvent.click(screen.getByTestId(FIRE_ESCAPE_TESTID));
expect(escapePreventDefault).toHaveBeenCalledTimes(1);
expect(screen.getByTestId("quick-chat-dialog")).toBeTruthy();
});
it("lets the second Escape close the modal once the clarification has collapsed", () => {
renderWithConversationTab();
fireEvent.click(screen.getByTestId(TOGGLE_ESCAPE_GUARD_TESTID)); // guard on: pending + expanded
fireEvent.click(screen.getByTestId(FIRE_ESCAPE_TESTID)); // 1st Escape: collapses, swallowed
fireEvent.click(screen.getByTestId(TOGGLE_ESCAPE_GUARD_TESTID)); // simulates the resulting collapse
fireEvent.click(screen.getByTestId(FIRE_ESCAPE_TESTID)); // 2nd Escape: unguarded, would close
expect(escapePreventDefault).toHaveBeenCalledTimes(1);
});
});
@@ -1,6 +1,6 @@
"use client";
import { memo, type CSSProperties } from "react";
import { memo, useState, type CSSProperties } from "react";
import { useTranslation } from "react-i18next";
import { Dialog, DialogContent, DialogTitle } from "@kandev/ui/dialog";
import { Button } from "@kandev/ui/button";
@@ -15,6 +15,7 @@ import { QuickTabAddMenu } from "./quick-tab-add-menu";
import { QuickChatSetup } from "./quick-chat-setup";
import { useQuickChatModal } from "./use-quick-chat-modal";
import { useQuickChatWidth } from "@/hooks/use-quick-chat-width";
import { ClarificationEscapeGuardProvider } from "@/hooks/use-clarification-escape-guard";
import { ConfigChatSetup } from "@/components/config-chat/config-chat-setup";
import { useConfigChat } from "@/components/config-chat/use-config-chat";
import type { QuickChatSession, QuickTerminalTab } from "@/lib/state/slices/ui/types";
@@ -240,14 +241,21 @@ export const QuickChatModal = memo(function QuickChatModal({ workspaceId }: Quic
const quickChat = useQuickChatModal(workspaceId, configChat.reset);
const setQuickChatInitialPrompt = useAppStore((state) => state.setQuickChatInitialPrompt);
const { width, leftResizeHandleProps, rightResizeHandleProps } = useQuickChatWidth();
const [clarificationEscapeGuarded, setClarificationEscapeGuarded] = useState(false);
return (
<>
<ClarificationEscapeGuardProvider value={setClarificationEscapeGuarded}>
<Dialog open={quickChat.isOpen} onOpenChange={quickChat.handleOpenChange}>
<DialogContent
className="!left-0 !top-0 !h-dvh !max-h-dvh !w-screen !max-w-none !translate-x-0 !translate-y-0 flex flex-col gap-0 p-0 pt-safe pb-safe shadow-2xl sm:!left-1/2 sm:!top-1/2 sm:!h-[85vh] sm:!max-h-[85vh] sm:!w-[var(--quick-chat-width)] sm:!max-w-[calc(100vw-2rem)] sm:!-translate-x-1/2 sm:!-translate-y-1/2"
style={{ "--quick-chat-width": `${width}px` } as CSSProperties}
showCloseButton={false}
overlayClassName="bg-black/20"
onEscapeKeyDown={(event) => {
// A pending, expanded clarification collapses first; the modal
// stays open. A second Escape (now unguarded) closes it, matching
// the main task chat panel's two-stage Escape after #2729.
if (clarificationEscapeGuarded) event.preventDefault();
}}
>
<DialogTitle className="sr-only">{t("common:commandQuickChat")}</DialogTitle>
<QuickChatResizeHandle edge="left" {...leftResizeHandleProps} />
@@ -266,6 +274,6 @@ export const QuickChatModal = memo(function QuickChatModal({ workspaceId }: Quic
onOpenChange={(open) => !open && quickChat.setSessionToClose(null)}
onConfirm={quickChat.handleConfirmClose}
/>
</>
</ClarificationEscapeGuardProvider>
);
});
@@ -0,0 +1,137 @@
import { createRef } from "react";
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import { render, cleanup, fireEvent, screen } from "@testing-library/react";
import { sessionId as toSessionId, taskId as toTaskId, type Message } from "@/lib/types/http";
import { ClarificationInputOverlay } from "./clarification-input-overlay";
vi.mock("@/lib/config", () => ({
getBackendConfig: () => ({ apiBaseUrl: "https://api.test" }),
}));
const mockUpdateMessage = vi.fn();
vi.mock("@/components/state-provider", () => ({
useAppStoreApi: () => ({
getState: () => ({ updateMessage: mockUpdateMessage }),
}),
}));
vi.mock("@kandev/ui/tooltip", () => ({
Tooltip: ({ children }: { children: React.ReactNode }) => <>{children}</>,
TooltipTrigger: ({ children }: { children: React.ReactNode }) => <>{children}</>,
TooltipContent: ({ children }: { children: React.ReactNode }) => <>{children}</>,
TooltipProvider: ({ children }: { children: React.ReactNode }) => <>{children}</>,
}));
const fetchMock = vi.fn();
function clarMessage(opts: {
id: string;
questionId: string;
index: number;
total: number;
}): Message {
return {
id: opts.id,
session_id: toSessionId("s1"),
task_id: toTaskId("t1"),
author_type: "agent",
content: "Q",
type: "clarification_request",
created_at: "2026-05-04T00:00:00Z",
metadata: {
pending_id: "p1",
question_id: opts.questionId,
question_index: opts.index,
question_total: opts.total,
status: "pending",
question: {
id: opts.questionId,
title: "",
prompt: `Question ${opts.index + 1}?`,
options: [{ option_id: "o1", label: "Option 1", description: "" }],
},
},
};
}
function renderOverlay(
messages: Message[],
overrides: Partial<{ onResolved: () => void; onDismiss: () => void }> = {},
) {
const scopeRef = createRef<HTMLDivElement>();
const onResolved = overrides.onResolved ?? vi.fn();
const onDismiss = overrides.onDismiss ?? vi.fn();
const utils = render(
<div ref={scopeRef} tabIndex={-1}>
<ClarificationInputOverlay
messages={messages}
onResolved={onResolved}
onDismiss={onDismiss}
shortcutScopeRef={scopeRef}
/>
</div>,
);
return { ...utils, scopeRef, onResolved, onDismiss };
}
beforeEach(() => {
fetchMock.mockReset();
mockUpdateMessage.mockReset();
fetchMock.mockResolvedValue(new Response(null, { status: 200 }));
globalThis.fetch = fetchMock as unknown as typeof globalThis.fetch;
});
afterEach(() => {
cleanup();
});
describe("ClarificationInputOverlay — Escape key", () => {
it("dismisses locally without posting a rejection, leaving the question pending", () => {
const messages = [clarMessage({ id: "m1", questionId: "q1", index: 0, total: 1 })];
const { onDismiss, scopeRef } = renderOverlay(messages);
fireEvent.keyDown(scopeRef.current!, { key: "Escape" });
expect(fetchMock).not.toHaveBeenCalled();
expect(onDismiss).toHaveBeenCalledTimes(1);
});
it("does not call skipAll (no store update to rejected) on Escape", () => {
const messages = [clarMessage({ id: "m1", questionId: "q1", index: 0, total: 1 })];
const { scopeRef } = renderOverlay(messages);
fireEvent.keyDown(scopeRef.current!, { key: "Escape" });
expect(mockUpdateMessage).not.toHaveBeenCalled();
});
it("still calls onDismiss from any step of a multi-question carousel", () => {
const messages = [
clarMessage({ id: "m1", questionId: "q1", index: 0, total: 2 }),
clarMessage({ id: "m2", questionId: "q2", index: 1, total: 2 }),
];
const { onDismiss, scopeRef } = renderOverlay(messages);
fireEvent.click(screen.getAllByTestId("clarification-step")[1]);
fireEvent.keyDown(scopeRef.current!, { key: "Escape" });
expect(fetchMock).not.toHaveBeenCalled();
expect(onDismiss).toHaveBeenCalledTimes(1);
});
});
describe("ClarificationInputOverlay — labelled Skip button", () => {
it("still POSTs a rejection when the Skip button is clicked", async () => {
const messages = [clarMessage({ id: "m1", questionId: "q1", index: 0, total: 1 })];
renderOverlay(messages);
fireEvent.click(screen.getByTestId("clarification-skip"));
await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1));
const [, init] = fetchMock.mock.calls[0];
expect(JSON.parse(String(init.body))).toEqual({
rejected: true,
reject_reason: "User skipped",
});
});
});
@@ -1,8 +1,7 @@
"use client";
import { useCallback, useEffect, useMemo, useRef, useState, type RefObject } from "react";
import { IconX, IconMessageQuestion, IconInfoCircle, IconCheck } from "@tabler/icons-react";
import { cn } from "@/lib/utils";
import { IconMessageQuestion, IconInfoCircle } from "@tabler/icons-react";
import ReactMarkdown from "react-markdown";
import { markdownComponents, remarkPlugins } from "@/components/shared/markdown-components";
import type {
@@ -12,14 +11,13 @@ import type {
ClarificationQuestion,
} from "@/lib/types/http";
import { useClarificationGroup } from "@/hooks/domains/session/use-clarification-group";
import { KeyboardShortcutTooltip } from "@/components/keyboard-shortcut-tooltip";
import { SHORTCUTS } from "@/lib/keyboard/constants";
import {
ClarificationCarouselNav,
ClarificationCustomInput,
ClarificationOptions,
ClarificationStepper,
} from "./clarification-overlay-parts";
import { ClarificationHeaderActions } from "./clarification-overlay-header";
import { useTranslation } from "react-i18next";
type ClarificationInputOverlayProps = {
@@ -27,6 +25,13 @@ type ClarificationInputOverlayProps = {
onResolved: () => void;
shortcutScopeRef: RefObject<HTMLElement | null>;
keyboardShortcutsEnabled?: boolean;
// Called when the user presses Escape. Unlike Skip, this must not answer or
// reject the bundle — it only dismisses the UI (e.g. collapses the panel).
// The question stays pending and the agent stays blocked.
onDismiss: () => void;
// Called by the expanded header's collapse control.
onCollapse?: () => void;
collapseContentId?: string;
};
type SingleQuestionMeta = {
@@ -187,7 +192,7 @@ type CarouselShortcutArgs = {
onPick: (index: number) => void;
onPrev: () => void;
onNext: () => void;
onSkip: () => void;
onDismiss: () => void;
onSubmit: () => void;
};
@@ -224,7 +229,7 @@ function CarouselKeyboardShortcuts(args: CarouselShortcutArgs) {
const { enabled, scopeRef } = args;
const optionsCount = args.meta.question.options.length;
const isLast = args.activeIndex === args.total - 1;
const { canSubmit, onPick, onPrev, onNext, onSkip, onSubmit } = args;
const { canSubmit, onPick, onPrev, onNext, onDismiss, onSubmit } = args;
useEffect(() => {
if (!enabled) return;
const onKey = (e: KeyboardEvent) => {
@@ -233,7 +238,7 @@ function CarouselKeyboardShortcuts(args: CarouselShortcutArgs) {
if (shouldIgnoreShortcut(e)) return;
if (e.key === "Escape") {
e.preventDefault();
onSkip();
onDismiss();
return;
}
if (e.key === "ArrowLeft") {
@@ -264,7 +269,7 @@ function CarouselKeyboardShortcuts(args: CarouselShortcutArgs) {
onPick,
onPrev,
onNext,
onSkip,
onDismiss,
onSubmit,
]);
return null;
@@ -282,6 +287,7 @@ type CarouselBodyProps = {
shortcutScopeRef: RefObject<HTMLElement | null>;
keyboardShortcutsEnabled: boolean;
onSubmit: () => void;
onDismiss: () => void;
};
type QuestionHandlerCtx = {
@@ -388,6 +394,7 @@ function ClarificationCarouselBody({
shortcutScopeRef,
keyboardShortcutsEnabled,
onSubmit,
onDismiss,
}: CarouselBodyProps) {
const total = sortedMessages.length;
const activeMessage = sortedMessages[Math.min(activeIndex, total - 1)] ?? null;
@@ -451,85 +458,21 @@ function ClarificationCarouselBody({
onPick={(idx) => onSelectOption(meta.question.options[idx].option_id)}
onPrev={() => setActiveIndex(Math.max(0, activeIndex - 1))}
onNext={() => setActiveIndex(Math.min(total - 1, activeIndex + 1))}
onSkip={() => void group.skipAll("User skipped")}
onDismiss={onDismiss}
onSubmit={onSubmit}
/>
</>
);
}
function ClarificationHeaderActions({
total,
allAnswered,
isSubmitting,
onSubmit,
onSkip,
}: {
total: number;
allAnswered: boolean;
isSubmitting: boolean;
onSubmit: () => void;
onSkip: () => void;
}) {
const { t } = useTranslation();
return (
<div className="flex items-center gap-2">
{total > 1 && (
<KeyboardShortcutTooltip
shortcut={SHORTCUTS.SUBMIT}
description={t("task:submitAnswers")}
enabled={!isSubmitting}
>
<span
className="inline-flex"
data-testid="clarification-submit-shortcut"
tabIndex={!allAnswered && !isSubmitting ? 0 : undefined}
>
<button
type="button"
onClick={onSubmit}
disabled={!allAnswered || isSubmitting}
data-testid="clarification-submit"
className={cn(
"inline-flex items-center gap-1 text-xs px-3 py-1 rounded font-medium transition-colors",
allAnswered && !isSubmitting
? "bg-blue-500 text-white hover:bg-blue-500/90 cursor-pointer"
: "bg-muted text-muted-foreground cursor-not-allowed",
)}
>
{isSubmitting ? t("task:submitting") : t("task:submit")}
<IconCheck className="h-3 w-3" />
</button>
</span>
</KeyboardShortcutTooltip>
)}
<KeyboardShortcutTooltip
shortcut={SHORTCUTS.CANCEL}
description={t("task:skipAllQuestions")}
enabled={!isSubmitting}
>
<span className="inline-flex" data-testid="clarification-skip-shortcut">
<button
type="button"
onClick={onSkip}
disabled={isSubmitting}
className="text-muted-foreground hover:text-foreground cursor-pointer disabled:opacity-50"
data-testid="clarification-skip"
aria-label={t("task:skipAllQuestions")}
>
<IconX className="h-4 w-4" />
</button>
</span>
</KeyboardShortcutTooltip>
</div>
);
}
export function ClarificationInputOverlay({
messages,
onResolved,
shortcutScopeRef,
keyboardShortcutsEnabled = true,
onDismiss,
onCollapse,
collapseContentId,
}: ClarificationInputOverlayProps) {
const sortedMessages = useMemo(
() => sortMessagesByQuestionIndex(resolveQuestionMessages(messages)),
@@ -565,8 +508,11 @@ export function ClarificationInputOverlay({
return (
<div className="relative" data-testid="clarification-overlay">
<div className="flex items-center justify-between gap-3 px-4 pt-2 pb-1">
<div className="flex items-center gap-3 min-w-0">
<div
className="flex min-h-11 items-center justify-between gap-3 px-4"
data-testid="clarification-overlay-header"
>
<div className="flex min-w-0 flex-1 items-center gap-3">
<IconMessageQuestion className="h-4 w-4 text-blue-500 flex-shrink-0" />
{total > 1 && (
<ClarificationStepper
@@ -580,7 +526,7 @@ export function ClarificationInputOverlay({
{total > 1 && (
<span
data-testid="clarification-group-progress"
className="text-xs text-muted-foreground"
className="min-w-0 truncate text-xs text-muted-foreground"
>
{group.answeredCount} of {group.total} answered
</span>
@@ -592,6 +538,8 @@ export function ClarificationInputOverlay({
isSubmitting={isSubmitting}
onSubmit={handleSubmit}
onSkip={() => void group.skipAll("User skipped")}
onCollapse={onCollapse}
collapseContentId={collapseContentId}
/>
</div>
{sharedContext && (
@@ -614,6 +562,7 @@ export function ClarificationInputOverlay({
shortcutScopeRef={shortcutScopeRef}
keyboardShortcutsEnabled={keyboardShortcutsEnabled}
onSubmit={handleSubmit}
onDismiss={onDismiss}
/>
</div>
);
@@ -0,0 +1,119 @@
"use client";
import { IconChevronDown, IconCheck, IconX } from "@tabler/icons-react";
import { Button } from "@kandev/ui/button";
import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip";
import { KeyboardShortcutTooltip } from "@/components/keyboard-shortcut-tooltip";
import { SHORTCUTS } from "@/lib/keyboard/constants";
import { cn } from "@/lib/utils";
import { useTranslation } from "react-i18next";
type ClarificationHeaderActionsProps = {
total: number;
allAnswered: boolean;
isSubmitting: boolean;
onSubmit: () => void;
onSkip: () => void;
onCollapse?: () => void;
collapseContentId?: string;
};
function ClarificationSkipButton({
isSubmitting,
onSkip,
label,
}: {
isSubmitting: boolean;
onSkip: () => void;
label: string;
}) {
const button = (
<span className="inline-flex" data-testid="clarification-skip-shortcut">
<button
type="button"
onClick={onSkip}
disabled={isSubmitting}
className="text-muted-foreground hover:text-foreground cursor-pointer disabled:opacity-50"
data-testid="clarification-skip"
aria-label={label}
>
<IconX className="h-4 w-4" />
</button>
</span>
);
if (isSubmitting) return button;
return (
<Tooltip>
<TooltipTrigger asChild>{button}</TooltipTrigger>
<TooltipContent>{label}</TooltipContent>
</Tooltip>
);
}
export function ClarificationHeaderActions({
total,
allAnswered,
isSubmitting,
onSubmit,
onSkip,
onCollapse,
collapseContentId,
}: ClarificationHeaderActionsProps) {
const { t } = useTranslation();
return (
<div className="flex shrink-0 items-center gap-2">
{total > 1 && (
<KeyboardShortcutTooltip
shortcut={SHORTCUTS.SUBMIT}
description={t("task:submitAnswers")}
enabled={!isSubmitting}
>
<span
className="inline-flex"
data-testid="clarification-submit-shortcut"
tabIndex={!allAnswered && !isSubmitting ? 0 : undefined}
>
<button
type="button"
onClick={onSubmit}
disabled={!allAnswered || isSubmitting}
data-testid="clarification-submit"
className={cn(
"inline-flex items-center gap-1 text-xs px-3 py-1 rounded font-medium transition-colors",
allAnswered && !isSubmitting
? "bg-blue-500 text-white hover:bg-blue-500/90 cursor-pointer"
: "bg-muted text-muted-foreground cursor-not-allowed",
)}
>
{isSubmitting ? t("task:submitting") : t("task:submit")}
<IconCheck className="h-3 w-3" />
</button>
</span>
</KeyboardShortcutTooltip>
)}
<ClarificationSkipButton
isSubmitting={isSubmitting}
onSkip={onSkip}
label={t("task:skipAllQuestions")}
/>
{onCollapse && (
<Button
type="button"
variant="ghost"
size="icon"
aria-label={t("chat:collapseClarification")}
aria-expanded={true}
aria-controls={collapseContentId}
title={t("chat:collapseClarification")}
data-testid="clarification-collapse-toggle"
onClick={onCollapse}
className="h-11 w-11 flex-shrink-0 cursor-pointer rounded-none"
>
<IconChevronDown className="h-4 w-4" />
</Button>
)}
</div>
);
}
@@ -42,7 +42,7 @@ export function ClarificationStepper({
const { t } = useTranslation();
return (
<div
className="flex items-center gap-1.5 select-none"
className="flex shrink-0 select-none items-center gap-1.5"
role="tablist"
data-testid="clarification-stepper"
>
@@ -0,0 +1,276 @@
import { createRef } from "react";
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import { render, cleanup, fireEvent, screen } from "@testing-library/react";
import { i18n } from "@/lib/i18n";
import { sessionId as toSessionId, taskId as toTaskId, type Message } from "@/lib/types/http";
import { ClarificationPanelSection } from "./clarification-panel-section";
vi.mock("@/lib/config", () => ({
getBackendConfig: () => ({ apiBaseUrl: "https://api.test" }),
}));
const mockUpdateMessage = vi.fn();
vi.mock("@/components/state-provider", () => ({
useAppStoreApi: () => ({
getState: () => ({ updateMessage: mockUpdateMessage }),
}),
}));
vi.mock("@kandev/ui/tooltip", () => ({
Tooltip: ({ children }: { children: React.ReactNode }) => <>{children}</>,
TooltipTrigger: ({ children }: { children: React.ReactNode }) => <>{children}</>,
TooltipContent: ({ children }: { children: React.ReactNode }) => <>{children}</>,
TooltipProvider: ({ children }: { children: React.ReactNode }) => <>{children}</>,
}));
const fetchMock = vi.fn();
function clarMessage(opts: {
pendingId: string;
id: string;
questionId: string;
index: number;
total: number;
}): Message {
return {
id: opts.id,
session_id: toSessionId("s1"),
task_id: toTaskId("t1"),
author_type: "agent",
content: "Q",
type: "clarification_request",
created_at: "2026-05-04T00:00:00Z",
metadata: {
pending_id: opts.pendingId,
question_id: opts.questionId,
question_index: opts.index,
question_total: opts.total,
status: "pending",
question: {
id: opts.questionId,
title: "",
prompt: `Question ${opts.index + 1}?`,
options: [{ option_id: "o1", label: "Option 1", description: "" }],
},
},
};
}
function renderSection(pending: boolean, messages: Message[], maxHeightVh = 50) {
const scopeRef = createRef<HTMLDivElement>();
const onResolved = vi.fn();
const utils = render(
<div ref={scopeRef} tabIndex={-1}>
<ClarificationPanelSection
pending={pending}
messages={messages}
onResolved={onResolved}
shortcutScopeRef={scopeRef}
maxHeightVh={maxHeightVh}
/>
</div>,
);
return { ...utils, scopeRef, onResolved };
}
beforeEach(() => {
fetchMock.mockReset();
mockUpdateMessage.mockReset();
fetchMock.mockResolvedValue(new Response(null, { status: 200 }));
globalThis.fetch = fetchMock as unknown as typeof globalThis.fetch;
});
afterEach(async () => {
cleanup();
// Restoring the locale is load-bearing: changeLanguage mutates the shared
// instance vitest.setup.ts initializes, so leaving it on pseudo would leak
// into every test that runs after this file.
await i18n.changeLanguage("en");
});
const SCROLL_REGION_TESTID = "clarification-scroll-region";
const CONTAINER_TESTID = "clarification-overlay-container";
const QUESTION_COUNT_TESTID = "clarification-question-count";
describe("ClarificationPanelSection — collapse affordance", () => {
it("keeps the expanded question controls in one header row", () => {
const messages = [
clarMessage({ pendingId: "p1", id: "m1", questionId: "q1", index: 0, total: 3 }),
clarMessage({ pendingId: "p1", id: "m2", questionId: "q2", index: 1, total: 3 }),
clarMessage({ pendingId: "p1", id: "m3", questionId: "q3", index: 2, total: 3 }),
];
renderSection(true, messages);
const header = screen.getByTestId("clarification-overlay-header");
expect(header.querySelector('[data-testid="clarification-stepper"]')).not.toBeNull();
expect(header.querySelector('[data-testid="clarification-submit"]')).not.toBeNull();
expect(header.querySelector('[data-testid="clarification-skip"]')).not.toBeNull();
expect(header.querySelector('[data-testid="clarification-collapse-toggle"]')).not.toBeNull();
expect(screen.queryByText("Clarification needed")).toBeNull();
});
it("collapses on Escape without posting a rejection, and stays reachable to expand again", () => {
const messages = [
clarMessage({ pendingId: "p1", id: "m1", questionId: "q1", index: 0, total: 1 }),
];
const { scopeRef } = renderSection(true, messages);
expect(screen.getByTestId(SCROLL_REGION_TESTID).className).not.toContain("hidden");
fireEvent.keyDown(scopeRef.current!, { key: "Escape" });
expect(fetchMock).not.toHaveBeenCalled();
expect(screen.queryByTestId(CONTAINER_TESTID)).not.toBeNull();
expect(screen.getByTestId(SCROLL_REGION_TESTID).className).toContain("hidden");
// Restore: clicking the toggle re-expands the same still-pending bundle.
fireEvent.click(screen.getByRole("button", { name: "Expand clarification" }));
expect(screen.getByTestId(SCROLL_REGION_TESTID).className).not.toContain("hidden");
});
it("the labelled Skip button inside an expanded section still posts a rejection", async () => {
const messages = [
clarMessage({ pendingId: "p1", id: "m1", questionId: "q1", index: 0, total: 1 }),
];
renderSection(true, messages);
fireEvent.click(screen.getByTestId("clarification-skip"));
await vi.waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1));
});
it("resets to expanded when a new bundle (different pending_id) arrives while collapsed", () => {
const bundleA = [
clarMessage({ pendingId: "p1", id: "m1", questionId: "q1", index: 0, total: 1 }),
];
const bundleB = [
clarMessage({ pendingId: "p2", id: "m2", questionId: "q2", index: 0, total: 1 }),
];
const { rerender, scopeRef } = renderSection(true, bundleA);
fireEvent.keyDown(scopeRef.current!, { key: "Escape" });
expect(screen.getByTestId(SCROLL_REGION_TESTID).className).toContain("hidden");
rerender(
<div ref={scopeRef} tabIndex={-1}>
<ClarificationPanelSection
pending={true}
messages={bundleB}
onResolved={vi.fn()}
shortcutScopeRef={scopeRef}
maxHeightVh={50}
/>
</div>,
);
expect(screen.getByTestId(SCROLL_REGION_TESTID).className).not.toContain("hidden");
});
});
describe("ClarificationPanelSection — per-surface max-height cap", () => {
it("renders the CSS max-height from the maxHeightVh prop instead of a hardcoded value", () => {
const messages = [
clarMessage({ pendingId: "p1", id: "m1", questionId: "q1", index: 0, total: 1 }),
];
renderSection(true, messages, 50);
const container = screen.getByTestId(CONTAINER_TESTID);
expect(container.style.maxHeight).toBe("50vh");
});
it("a different surface's maxHeightVh renders a different cap (e.g. Quick Chat's 35vh)", () => {
const messages = [
clarMessage({ pendingId: "p1", id: "m1", questionId: "q1", index: 0, total: 1 }),
];
renderSection(true, messages, 35);
const container = screen.getByTestId(CONTAINER_TESTID);
expect(container.style.maxHeight).toBe("35vh");
});
});
describe("ClarificationPanelSection — question-count localization", () => {
it("uses the singular form for exactly one question in the bundle", () => {
const messages = [
clarMessage({ pendingId: "p1", id: "m1", questionId: "q1", index: 0, total: 2 }),
];
const { scopeRef } = renderSection(true, messages);
fireEvent.keyDown(scopeRef.current!, { key: "Escape" });
expect(screen.getByTestId(QUESTION_COUNT_TESTID).textContent).toBe("1 question");
});
it("uses the plural form for more than one question in the bundle", () => {
const messages = [
clarMessage({ pendingId: "p1", id: "m1", questionId: "q1", index: 0, total: 2 }),
clarMessage({ pendingId: "p1", id: "m2", questionId: "q2", index: 1, total: 2 }),
];
const { scopeRef } = renderSection(true, messages);
fireEvent.keyDown(scopeRef.current!, { key: "Escape" });
expect(screen.getByTestId(QUESTION_COUNT_TESTID).textContent).toBe("2 questions");
});
it("keeps the total count wording after a partial answer", () => {
const messages = [
clarMessage({ pendingId: "p1", id: "m1", questionId: "q1", index: 0, total: 2 }),
clarMessage({ pendingId: "p1", id: "m2", questionId: "q2", index: 1, total: 2 }),
];
const { scopeRef } = renderSection(true, messages);
fireEvent.click(screen.getAllByTestId("clarification-option")[0]);
fireEvent.keyDown(scopeRef.current!, { key: "Escape" });
expect(screen.getByTestId(QUESTION_COUNT_TESTID).textContent).toBe("2 questions");
});
it("resolves the question-count text through the catalog on a locale switch, not an English fallback", async () => {
const messages = [
clarMessage({ pendingId: "p1", id: "m1", questionId: "q1", index: 0, total: 1 }),
];
const { scopeRef } = renderSection(true, messages);
fireEvent.keyDown(scopeRef.current!, { key: "Escape" });
await i18n.changeLanguage("pseudo");
expect(screen.getByTestId(QUESTION_COUNT_TESTID).textContent).toBe("1 qũēśţĩōń");
});
});
describe("ClarificationPanelSection — dragged height does not survive a bundle handover", () => {
it("resets the user-dragged height when a new bundle (different pending_id) replaces one still pending", () => {
const bundleA = [
clarMessage({ pendingId: "p1", id: "m1", questionId: "q1", index: 0, total: 1 }),
];
const bundleB = [
clarMessage({ pendingId: "p2", id: "m2", questionId: "q2", index: 0, total: 1 }),
];
const { rerender, scopeRef } = renderSection(true, bundleA);
const container = screen.getByTestId(CONTAINER_TESTID);
const handle = screen.getByRole("button", { name: /resize/i });
fireEvent.mouseDown(handle, { clientY: 500 });
fireEvent.mouseMove(document, { clientY: 300 }); // drag up 200px → explicit pixel height
fireEvent.mouseUp(document);
expect(container.style.height).toMatch(/^\d+(\.\d+)?px$/);
rerender(
<div ref={scopeRef} tabIndex={-1}>
<ClarificationPanelSection
pending={true}
messages={bundleB}
onResolved={vi.fn()}
shortcutScopeRef={scopeRef}
maxHeightVh={50}
/>
</div>,
);
// Bundle B must not inherit bundle A's dragged pixel height.
expect(container.style.height).toBe("");
});
});
@@ -0,0 +1,160 @@
"use client";
import { useEffect, useId, useRef, useState, type RefObject } from "react";
import { IconChevronDown, IconChevronUp, IconMessageQuestion } from "@tabler/icons-react";
import { Button } from "@kandev/ui/button";
import { useTranslation } from "react-i18next";
import { ClarificationInputOverlay } from "./clarification-input-overlay";
import { ResizeHandle } from "./resize-handle";
import { useResizableClarificationOverlay } from "@/hooks/use-resizable-clarification-overlay";
import { useClarificationEscapeGuard } from "@/hooks/use-clarification-escape-guard";
import type { ClarificationRequestMetadata, Message } from "@/lib/types/http";
type ClarificationPanelSectionProps = {
pending: boolean;
messages: readonly Message[] | null | undefined;
onResolved: () => void;
shortcutScopeRef: RefObject<HTMLElement | null>;
/**
* Caps the expanded overlay's height as a percentage of the viewport.
* Drives both the rendered CSS max-height and the resize hook's drag
* clamp — the two MUST agree, or the drag handle silently stops
* responding before its own visible ceiling.
*/
maxHeightVh: number;
};
function pendingIdFromMessages(messages: readonly Message[] | null | undefined): string | null {
const first = messages?.[0];
if (!first) return null;
const metadata = first.metadata as ClarificationRequestMetadata | undefined;
return metadata?.pending_id ?? null;
}
// Tracks collapse state per bundle: a bundle the user dismissed stays
// collapsed while it's still pending, but a *new* bundle (different
// pending_id) always opens expanded, even if the previous one never resolved.
function useCollapsedForBundle(pendingId: string | null) {
const [collapsed, setCollapsed] = useState(false);
const lastPendingId = useRef(pendingId);
useEffect(() => {
if (pendingId !== lastPendingId.current) {
lastPendingId.current = pendingId;
setCollapsed(false);
}
}, [pendingId]);
return [collapsed, setCollapsed] as const;
}
/**
* Collapsible clarification section shared by the task chat panel and Quick
* Chat. Escape (or the labelled toggle) collapses this to a persistent header
* bar instead of answering or rejecting the bundle, so a blocked agent stays
* visibly blocked and the question stays reachable until answered or skipped.
*/
export function ClarificationPanelSection({
pending,
messages,
onResolved,
shortcutScopeRef,
maxHeightVh,
}: ClarificationPanelSectionProps) {
const { t } = useTranslation();
const pendingId = pendingIdFromMessages(messages);
const [collapsed, setCollapsed] = useCollapsedForBundle(pendingId);
const contentId = useId();
const { height, containerRef, resetHeight, resizeHandleProps } = useResizableClarificationOverlay(
maxHeightVh / 100,
);
// A newly opened clarification starts expanded and auto-sized. Collapsing
// an active one leaves its form state and user-selected height intact.
useEffect(() => {
if (!pending) resetHeight();
}, [pending, resetHeight]);
// A new bundle (different pending_id) replacing a still-pending one must
// not inherit the previous bundle's dragged height.
useEffect(() => {
resetHeight();
}, [pendingId, resetHeight]);
// While this panel is expanded and pending, tell an ancestor dialog (Quick
// Chat) to swallow Escape instead of closing over it, so the first Escape
// collapses the panel like it does in the non-modal task chat panel.
useClarificationEscapeGuard(pending && !collapsed);
if (!pending) return null;
const questionCount = messages?.length ?? 0;
const actionLabel = collapsed ? t("chat:expandClarification") : t("chat:collapseClarification");
const compact = collapsed || questionCount === 0;
return (
<div className="relative flex-shrink-0 border-t border-sky-400/30 bg-card">
{!compact && <ResizeHandle {...resizeHandleProps} />}
<div
ref={containerRef}
data-testid="clarification-overlay-container"
className={
compact ? "h-11" : "flex min-h-[7.5rem] flex-col overflow-hidden overscroll-contain"
}
style={
compact
? undefined
: { maxHeight: `${maxHeightVh}vh`, ...(height !== null ? { height } : {}) }
}
>
{compact && (
<div className="flex h-11 flex-shrink-0 items-center justify-between gap-2 pl-4">
<div className="flex min-w-0 items-center gap-2 text-sm font-medium">
<IconMessageQuestion className="h-4 w-4 flex-shrink-0 text-blue-500" />
<span className="truncate">{t("chat:clarificationNeeded")}</span>
{questionCount > 0 && (
<span
data-testid="clarification-question-count"
className="truncate text-xs font-normal text-muted-foreground"
>
{t("task:questionCount", { count: questionCount })}
</span>
)}
</div>
<Button
type="button"
variant="ghost"
size="icon"
className="h-11 w-11 flex-shrink-0 cursor-pointer rounded-none"
aria-label={actionLabel}
aria-expanded={!collapsed}
aria-controls={contentId}
title={actionLabel}
data-testid="clarification-collapse-toggle"
onClick={() => setCollapsed((current) => !current)}
>
{collapsed ? (
<IconChevronUp className="h-4 w-4" />
) : (
<IconChevronDown className="h-4 w-4" />
)}
</Button>
</div>
)}
<div
id={contentId}
data-testid="clarification-scroll-region"
className={compact ? "hidden" : "min-h-0 flex-1 overflow-y-auto px-1"}
>
<ClarificationInputOverlay
messages={messages}
onResolved={onResolved}
shortcutScopeRef={shortcutScopeRef}
keyboardShortcutsEnabled={!collapsed}
onDismiss={() => setCollapsed(true)}
onCollapse={() => setCollapsed(true)}
collapseContentId={contentId}
/>
</div>
</div>
</div>
);
}
+10 -67
View File
@@ -22,10 +22,8 @@ import { useResponsiveBreakpoint } from "@/hooks/use-responsive-breakpoint";
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 { ClarificationPanelSection } from "./chat/clarification-panel-section";
import { useComposerAgentStartHint } from "./chat/use-composer-agent-start-hint";
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";
@@ -35,7 +33,6 @@ import { findUnreadDividerItemId, lastRenderedMessageId } from "@/lib/session-un
import { useSessionReadTracking } from "./chat/use-session-read-tracking";
import { useDrainOlderMessages } from "@/components/task/chat/use-drain-older-messages";
import { useAppStore } from "@/components/state-provider";
import type { Message } from "@/lib/types/http";
import { getSessionWorkspacePath } from "@/lib/session-workspace-path";
import { routePanelMouseDown } from "./chat/route-panel-mouse-down";
import { useTranslation } from "react-i18next";
@@ -250,18 +247,6 @@ export const TaskChatPanel = memo(function TaskChatPanel({
);
const { handleCancelTurn } = useChatPanelHandlers(resolvedSessionId, 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<HTMLDivElement>(null);
const messageListRef = useRef<MessageListHandle>(null);
@@ -402,16 +387,15 @@ export const TaskChatPanel = memo(function TaskChatPanel({
/>
<SessionSearchOverlay search={search} agentLabel={agentLabel} agentName={agentName} />
</PanelBody>
<ClarificationSection
pendingClarification={Boolean(pendingClarification)}
isArchived={isArchived}
containerRef={clarificationContainerRef}
resizeProps={clarificationResizeProps}
height={clarificationHeight}
messages={pendingClarificationGroup}
onResolved={handleClarificationResolved}
shortcutScopeRef={panelRef}
/>
{!isArchived && (
<ClarificationPanelSection
pending={Boolean(pendingClarification)}
messages={pendingClarificationGroup}
onResolved={handleClarificationResolved}
shortcutScopeRef={panelRef}
maxHeightVh={50}
/>
)}
<ChatFooter
isArchived={isArchived}
chatInputRef={chatInputRef}
@@ -436,47 +420,6 @@ export const TaskChatPanel = memo(function TaskChatPanel({
);
});
type ClarificationSectionProps = {
pendingClarification: boolean;
isArchived: boolean;
containerRef: RefObject<HTMLDivElement | null>;
resizeProps: { onMouseDown: (e: React.MouseEvent) => void; onDoubleClick: () => void };
height: number | null;
messages: readonly Message[] | null | undefined;
onResolved: () => void;
shortcutScopeRef: RefObject<HTMLElement | null>;
};
function ClarificationSection({
pendingClarification,
isArchived,
containerRef,
resizeProps,
height,
messages,
onResolved,
shortcutScopeRef,
}: ClarificationSectionProps) {
if (!pendingClarification || isArchived) return null;
return (
<div className="relative flex-shrink-0 border-t border-sky-400/30 bg-card">
<ResizeHandle {...resizeProps} />
<div
ref={containerRef}
data-testid="clarification-overlay-container"
className="px-1 overflow-y-scroll overscroll-contain max-h-[50vh]"
style={height === null ? undefined : { height }}
>
<ClarificationInputOverlay
messages={messages}
onResolved={onResolved}
shortcutScopeRef={shortcutScopeRef}
/>
</div>
</div>
);
}
type ChatFooterProps = {
isArchived: boolean;
chatInputRef: RefObject<
+16
View File
@@ -474,6 +474,22 @@ export class SessionPage {
return this.activeChat().getByTestId("clarification-overlay");
}
/**
* The persistent bar wrapping the clarification overlay. Stays mounted
* (collapsed to a header row) while the bundle is pending, even after the
* user dismisses it with Escape or the collapse toggle.
*/
clarificationBar(): Locator {
return this.activeChat().getByTestId("clarification-overlay-container");
}
/** Expand/collapse toggle in the clarification bar's header row. */
clarificationCollapseToggle(): Locator {
// The expanded overlay stays mounted but is hidden when the compact bar
// is shown, so scope this locator to the one visible toggle.
return this.activeChat().locator('[data-testid="clarification-collapse-toggle"]:visible');
}
/** Shared context shown once above the active clarification question. */
clarificationContext(): Locator {
return this.clarificationOverlay().getByTestId("clarification-context");
@@ -78,8 +78,8 @@ test.describe("Clarification overlay resizable layout", () => {
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 });
// without bumping into the 35vh safety cap on the first drag.
await testPage.setViewportSize({ width: 1280, height: 3000 });
const session = await seedTaskAndWaitForIdle(
testPage,
@@ -94,7 +94,10 @@ test.describe("Clarification overlay resizable layout", () => {
const container = testPage.getByTestId("clarification-overlay-container");
await expect(container).toBeVisible();
const scrollRegion = container;
// The container now also carries the persistent header row (collapse
// toggle + waiting count), so it no longer scrolls itself — the nested
// scroll region below the header does.
const scrollRegion = container.getByTestId("clarification-scroll-region");
const initial = await container.evaluate((el) => {
return {
height: el.getBoundingClientRect().height,
@@ -102,13 +105,13 @@ test.describe("Clarification overlay resizable layout", () => {
inlineHeight: (el as HTMLElement).style.height,
};
});
await expect(scrollRegion).toHaveCSS("overflow-y", "scroll");
await expect(scrollRegion).toHaveCSS("overflow-y", "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.
// question card but well under the 35vh safety cap.
expect(initial.height).toBeGreaterThan(200);
expect(initial.height).toBeLessThan(1000 * 0.5);
expect(initial.height).toBeLessThan(3000 * 0.35);
const handle = container.locator("xpath=..").locator("button[aria-label='Resize']");
await expect(handle).toBeVisible();
@@ -142,4 +145,61 @@ test.describe("Clarification overlay resizable layout", () => {
expect(afterReset.inlineHeight).toBe("");
expect(Math.abs(afterReset.height - initial.height)).toBeLessThanOrEqual(2);
});
// Regression coverage for the task-chat-panel cap: the rendered CSS
// max-height and the resize hook's drag clamp must agree on 50vh here.
// Before the fix, ClarificationPanelSection rendered a flat 35vh cap
// (inherited from Quick Chat) while the hook still clamped drags to 50vh,
// so the handle silently stopped growing the box ~30% short of its own
// documented ceiling with zero feedback.
test("dragging well past the old 35vh mark keeps growing the task panel up to its 50vh cap, with no dead zone", async ({
testPage,
apiClient,
seedData,
}) => {
// 2000px viewport → 35vh = 700px, 50vh = 1000px. Round numbers make the
// two caps unambiguous to distinguish in assertions.
await testPage.setViewportSize({ width: 1280, height: 2000 });
const session = await seedTaskAndWaitForIdle(
testPage,
apiClient,
seedData,
"Clarification Resize Cap",
);
await session.sendMessage("/ask-multiple");
await expect(session.clarificationOverlay()).toBeVisible({ timeout: 30_000 });
const container = session.clarificationBar();
await expect(container).toBeVisible();
await expect(container).toHaveCSS("max-height", "1000px");
const handle = container.locator("xpath=..").locator("button[aria-label='Resize']");
await expect(handle).toBeVisible();
// Drag far enough (900px) that a working cap must clamp it — well past
// the old, wrong 700px (35vh) ceiling.
const handleBox = await handle.boundingBox();
expect(handleBox).not.toBeNull();
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 - 900, { steps: 15 });
await testPage.mouse.up();
const afterDrag = await container.evaluate((el) => ({
height: el.getBoundingClientRect().height,
inlineHeight: (el as HTMLElement).style.height,
}));
// The drag target overshoots 50vh, so the hook's own clamp pins the
// inline height at exactly 1000px...
expect(afterDrag.inlineHeight).toBe("1000px");
// ...and because the CSS cap now agrees, the box actually renders there
// instead of stopping dead at the old 700px ceiling.
expect(afterDrag.height).toBeGreaterThan(700);
expect(afterDrag.height).toBeCloseTo(1000, 0);
});
});
+216 -4
View File
@@ -5,6 +5,7 @@ import type { SeedData } from "../../fixtures/test-base";
import type { ApiClient } from "../../helpers/api-client";
import { seedClarificationSession } from "../../helpers/clarification";
import { waitForSessionState } from "../../helpers/session";
import { dwell } from "../../helpers/causal-waits";
import { SessionPage } from "../../pages/session-page";
import { KanbanPage } from "../../pages/kanban-page";
import { SidebarFilterPopoverPage } from "../../pages/sidebar-filter-popover";
@@ -306,6 +307,84 @@ test.describe("Clarification flow", () => {
await expect(session.chat).toContainText("Use the embedded database for this task");
});
test("Escape after the clarification detaches still doesn't reject, and the question stays answerable", async ({
testPage,
apiClient,
seedData,
}) => {
const workflow = await apiClient.createWorkflow(
seedData.workspaceId,
"Clarification Timeout Esc Workflow",
);
const timeoutStep = await apiClient.createWorkflowStep(workflow.id, "Timeout Step", 0);
const task = await apiClient.createTaskWithAgent(
seedData.workspaceId,
"Clarification Timeout Esc",
seedData.agentProfileId,
{
description: "/e2e:clarification-timeout",
workflow_id: workflow.id,
workflow_step_id: timeoutStep.id,
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 expect(session.clarificationOverlay()).toBeVisible({ timeout: 30_000 });
await expect(session.chat).toContainText("Question timed out", { timeout: 30_000 });
await expect(session.clarificationDeferredNotice()).toBeVisible({ timeout: 10_000 });
await waitForSessionState(apiClient, {
taskId: task.id,
sessionId: task.session_id,
expectedState: "WAITING_FOR_INPUT",
message: "timed-out clarification session must remain ready for a deferred answer",
timeout: 30_000,
});
let respondCalls = 0;
await testPage.route("**/api/v1/clarification/*/respond", async (route) => {
respondCalls += 1;
await route.continue();
});
// The MCP wait already detached (agent_disconnected cleanup ran), but
// Escape must still be a local-only dismiss — not a rejection — and the
// deferred question must stay answerable after restoring it.
await session.chat.focus();
await testPage.keyboard.press("Escape");
await expect(session.clarificationOverlay()).not.toBeVisible();
await expect(session.clarificationBar()).toBeVisible();
await dwell(
testPage,
300,
"negative-assertion",
"asserts Escape never posts a rejection after detach; there is no event for the request that must not fire",
);
expect(respondCalls).toBe(0);
await session.clarificationCollapseToggle().click();
await expect(session.clarificationOverlay()).toBeVisible();
const input = session.clarificationInput();
const inputRow = session.clarificationCustomInput();
await expect(input).toBeEnabled();
await inputRow.click({ position: { x: 4, y: 4 } });
await expect(input).toBeFocused();
await input.pressSequentially("Use the embedded database for this task");
await input.press("Enter");
await expect(session.idleInput()).toBeVisible({ timeout: 30_000 });
await expect(session.chat).toContainText("Use the embedded database for this task");
});
test("options render label and description on separate rows", async ({
testPage,
apiClient,
@@ -765,9 +844,12 @@ test.describe("Multi-question clarification carousel", () => {
await expect(submitTooltip).toContainText(/Ctrl|⌘/);
await expect(submitTooltip).toContainText("Enter");
// Skip has no keyboard shortcut: Escape only dismisses the bundle locally
// (see the "Escape dismisses" tests below), so its tooltip must not claim "Esc".
await testPage.getByTestId("clarification-skip-shortcut").hover();
const skipTooltip = testPage.getByRole("tooltip", { name: /Skip all questions/ });
await expect(skipTooltip).toContainText("Esc");
await expect(skipTooltip).toBeVisible();
await expect(skipTooltip).not.toContainText("Esc");
await session.clarificationOption("PostgreSQL").click();
await session.clarificationPrev().hover();
@@ -820,7 +902,7 @@ test.describe("Multi-question clarification carousel", () => {
await expect(session.clarificationOverlay()).not.toBeVisible({ timeout: 30_000 });
});
test("Esc skips the entire bundle from anywhere in the carousel", async ({
test("Escape dismisses the bundle locally from anywhere in the carousel, without rejecting it", async ({
testPage,
apiClient,
seedData,
@@ -829,17 +911,147 @@ test.describe("Multi-question clarification carousel", () => {
testPage,
apiClient,
seedData,
"Multi-q esc",
"Multi-q esc dismiss",
"clarification-multi",
);
await expect(session.clarificationOverlay()).toBeVisible({ timeout: 30_000 });
let respondCalls = 0;
await testPage.route("**/api/v1/clarification/*/respond", async (route) => {
respondCalls += 1;
await route.continue();
});
await session.clarificationStep(1).click();
await testPage.keyboard.press("Escape");
// The overlay collapses locally, but the bar stays put — the bundle is
// still pending and reachable, not answered or rejected.
await expect(session.clarificationOverlay()).not.toBeVisible();
await expect(session.clarificationBar()).toBeVisible();
await expect(session.clarificationCollapseToggle()).toBeVisible();
await dwell(
testPage,
300,
"negative-assertion",
"asserts Escape never posts a rejection; there is no event for the request that must not fire",
);
expect(respondCalls).toBe(0);
await expect(session.chat).not.toContainText("rejected");
});
test("Escape leaves the agent blocked on the pending clarification", async ({
testPage,
apiClient,
seedData,
}) => {
const task = await apiClient.createTaskWithAgent(
seedData.workspaceId,
"Clarification esc keeps agent blocked",
seedData.agentProfileId,
{
description: "/e2e:clarification-multi",
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 expect(session.clarificationOverlay()).toBeVisible({ timeout: 30_000 });
const { sessions: before } = await apiClient.listTaskSessions(task.id);
const stateBeforeEscape = before.find((s) => s.id === task.session_id)?.state;
if (!stateBeforeEscape) throw new Error("expected to find the seeded session's state");
await session.chat.focus();
await testPage.keyboard.press("Escape");
await expect(session.clarificationOverlay()).not.toBeVisible();
await dwell(
testPage,
500,
"negative-assertion",
"asserts Escape never resumes the blocked turn; there is no event for the state transition that must not happen",
);
const { sessions: after } = await apiClient.listTaskSessions(task.id);
expect(after.find((s) => s.id === task.session_id)?.state).toBe(stateBeforeEscape);
});
test("dismissing with Escape and restoring lets the user answer normally", async ({
testPage,
apiClient,
seedData,
}) => {
const session = await seedClarificationTask(
testPage,
apiClient,
seedData,
"Multi-q esc restore answer",
"clarification-multi",
);
await expect(session.clarificationOverlay()).toBeVisible({ timeout: 30_000 });
await session.chat.focus();
await testPage.keyboard.press("Escape");
await expect(session.clarificationOverlay()).not.toBeVisible();
await expect(session.clarificationBar()).toBeVisible();
await session.clarificationCollapseToggle().click();
await expect(session.clarificationOverlay()).toBeVisible();
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.chat).toContainText("rejected");
await expect(session.idleInput()).toBeVisible({ timeout: 30_000 });
});
test("dismissing mid-bundle preserves recorded answers through the round trip", async ({
testPage,
apiClient,
seedData,
}) => {
const session = await seedClarificationTask(
testPage,
apiClient,
seedData,
"Multi-q esc preserves answers",
"clarification-multi",
);
await expect(session.clarificationOverlay()).toBeVisible({ timeout: 30_000 });
await session.clarificationOption("PostgreSQL").click();
await expect(session.clarificationStep(0)).toHaveAttribute("data-answered", "true");
await expect(session.clarificationGroupProgress()).toContainText("1 of 3 answered");
await session.chat.focus();
await testPage.keyboard.press("Escape");
await expect(session.clarificationOverlay()).not.toBeVisible();
await session.clarificationCollapseToggle().click();
await expect(session.clarificationOverlay()).toBeVisible();
// Both the stepper's answered mark and the recorded answer itself must
// survive the dismiss/restore round trip.
await expect(session.clarificationStep(0)).toHaveAttribute("data-answered", "true");
await expect(session.clarificationGroupProgress()).toContainText("1 of 3 answered");
await session.clarificationStep(0).click();
const selectedOption = session
.clarificationQuestionCardById("db")
.locator('[data-testid="clarification-option"][data-selected="true"]');
await expect(selectedOption).toContainText("PostgreSQL");
});
test("Back button is disabled on the first step", async ({ testPage, apiClient, seedData }) => {
@@ -63,6 +63,38 @@ test.describe("Quick Chat", () => {
).toHaveAttribute("data-active", "true");
});
test("Escape collapses the clarification panel before closing the modal", async ({
testPage,
}) => {
const dialog = await openQuickChatWithAgent(testPage);
await sendQuickChatMessage(dialog, testPage, "/e2e:clarification-multi");
const clarification = dialog.getByTestId("clarification-overlay");
await expect(clarification).toBeVisible({ timeout: 30_000 });
const bar = dialog.getByTestId("clarification-overlay-container");
// The collapse shortcut only fires for keydowns targeting the shortcut
// scope (quick-chat-content), same as the numeric-step shortcut above.
await dialog.getByTestId("quick-chat-messages").click({ position: { x: 8, y: 8 } });
await expect(dialog.getByTestId("quick-chat-content")).toBeFocused();
await testPage.keyboard.press("Escape");
// First Escape: a pending, expanded clarification collapses in place and
// the modal stays open, matching the main task chat panel's two-stage
// Escape after #2729 — this is what distinguishes it from Radix's default
// (close-on-Escape) DismissableLayer behavior.
await expect(dialog).toBeVisible();
await expect(clarification).not.toBeVisible();
await expect(bar).toBeVisible();
await testPage.keyboard.press("Escape");
// Second Escape is now unguarded: it closes the whole modal.
await expect(dialog).not.toBeVisible();
});
test("offers configuration chat in setup and hides it once one exists", async ({
testPage,
apiClient,
@@ -0,0 +1,35 @@
import { describe, it, expect, vi } from "vitest";
import { renderHook } from "@testing-library/react";
import {
ClarificationEscapeGuardProvider,
useClarificationEscapeGuard,
} from "./use-clarification-escape-guard";
describe("useClarificationEscapeGuard", () => {
it("does nothing outside a provider, so non-modal callers can invoke it unconditionally", () => {
expect(() => renderHook(() => useClarificationEscapeGuard(true))).not.toThrow();
});
it("reports guarded state changes to the provider, and clears the guard on unmount", () => {
const setGuarded = vi.fn();
const { rerender, unmount } = renderHook(
({ guarded }) => useClarificationEscapeGuard(guarded),
{
initialProps: { guarded: false },
wrapper: ({ children }) => (
<ClarificationEscapeGuardProvider value={setGuarded}>
{children}
</ClarificationEscapeGuardProvider>
),
},
);
expect(setGuarded).toHaveBeenLastCalledWith(false);
rerender({ guarded: true });
expect(setGuarded).toHaveBeenLastCalledWith(true);
unmount();
expect(setGuarded).toHaveBeenLastCalledWith(false);
});
});
@@ -0,0 +1,25 @@
"use client";
import { createContext, useContext, useEffect } from "react";
type ClarificationEscapeGuardSetter = (guarded: boolean) => void;
const ClarificationEscapeGuardContext = createContext<ClarificationEscapeGuardSetter | null>(null);
export const ClarificationEscapeGuardProvider = ClarificationEscapeGuardContext.Provider;
/**
* Tells an ancestor dialog (via ClarificationEscapeGuardProvider) to swallow
* its Escape-closes-dialog default while `guarded` is true, so the first
* Escape collapses this panel instead of dismissing the surrounding modal.
* No-ops outside a provider, so callers like the non-modal task chat panel
* can invoke this unconditionally.
*/
export function useClarificationEscapeGuard(guarded: boolean) {
const setGuarded = useContext(ClarificationEscapeGuardContext);
useEffect(() => {
if (!setGuarded) return;
setGuarded(guarded);
return () => setGuarded(false);
}, [guarded, setGuarded]);
}
@@ -81,7 +81,7 @@ describe("useResizableClarificationOverlay", () => {
expect(result.current.height).toBe(MIN_HEIGHT);
});
it("clamps above 50% of viewport when dragging up past it", () => {
it("clamps above 50% of viewport when dragging up past it (default ratio)", () => {
setInnerHeight(800); // 50% = 400 cap
const { result } = renderHook(() => useResizableClarificationOverlay());
attachContainerWithHeight(result.current.containerRef, 300);
@@ -170,3 +170,27 @@ describe("useResizableClarificationOverlay", () => {
expect(result.current.height).toBeNull();
});
});
describe("useResizableClarificationOverlay — maxViewportRatio parameter", () => {
beforeEach(() => {
setInnerHeight(1000);
document.body.style.cursor = "";
document.body.style.userSelect = "";
});
afterEach(() => {
document.body.style.cursor = "";
document.body.style.userSelect = "";
});
it("honors a caller-supplied maxViewportRatio instead of the 50% default", () => {
setInnerHeight(800); // 35% = 280 cap, well below the 400 the default would allow
const { result } = renderHook(() => useResizableClarificationOverlay(0.35));
attachContainerWithHeight(result.current.containerRef, 300);
act(() => result.current.resizeHandleProps.onMouseDown(fakeReactMouseEvent(500)));
act(() => dispatchMouseMove(0)); // delta=500 → 800 requested, must clamp to 280 (35%), not 400 (50%)
expect(result.current.height).toBe(280);
});
});
@@ -3,14 +3,14 @@
import { useCallback, useEffect, useRef, useState } from "react";
const MIN_HEIGHT = 120;
const MAX_VIEWPORT_RATIO = 0.5;
const DEFAULT_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);
function clampHeight(value: number, maxViewportRatio: number): number {
const max = Math.round(viewportHeight() * maxViewportRatio);
return Math.min(Math.max(value, MIN_HEIGHT), max);
}
@@ -20,14 +20,20 @@ function clampHeight(value: number): number {
* 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.
* MIN_HEIGHT and `maxViewportRatio` (default 50%) of the viewport.
* Double-clicking the handle returns to the auto-sized default.
*
* `maxViewportRatio` MUST match whatever CSS `max-height` the caller renders
* on the same container — a mismatch creates a dead zone where the drag
* keeps computing a larger height than CSS will actually render.
*
* 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() {
export function useResizableClarificationOverlay(
maxViewportRatio: number = DEFAULT_MAX_VIEWPORT_RATIO,
) {
// null = auto-size to content; number = user-driven pixel height.
const [height, setHeight] = useState<number | null>(null);
const containerRef = useRef<HTMLDivElement>(null);
@@ -55,7 +61,7 @@ export function useResizableClarificationOverlay() {
// 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));
setHeight(clampHeight(startHeight.current + delta, maxViewportRatio));
};
const handleMouseUp = () => {
if (!isDragging.current) return;
@@ -74,7 +80,7 @@ export function useResizableClarificationOverlay() {
document.body.style.userSelect = "";
}
};
}, []);
}, [maxViewportRatio]);
return {
height,