fix(web): keep composer usable during clarifications (#1916)
* fix(web): keep composer usable during clarifications * fix(web): guard clarification shortcuts in editor
This commit is contained in:
@@ -70,7 +70,7 @@ vi.mock("@/lib/ws/connection", () => ({
|
||||
getWebSocketClient: () => ({ send: vi.fn() }),
|
||||
}));
|
||||
|
||||
import { useSubmitHandler } from "./chat-input-area";
|
||||
import { resolveInputPlaceholder, useSubmitHandler } from "./chat-input-area";
|
||||
|
||||
beforeEach(() => {
|
||||
handleSendMessageMock.mockReset();
|
||||
@@ -83,31 +83,40 @@ afterEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
describe("useSubmitHandler", () => {
|
||||
function panelState(overrides = {}) {
|
||||
return {
|
||||
resolvedSessionId: "session-1",
|
||||
taskId: "task-1",
|
||||
sessionModel: null,
|
||||
activeModel: null,
|
||||
isAgentBusy: false,
|
||||
activeDocument: null,
|
||||
planComments: [],
|
||||
pendingPRFeedback: [],
|
||||
walkthroughComments: [],
|
||||
contextFiles: [],
|
||||
prompts: [],
|
||||
markCommentsSent: vi.fn(),
|
||||
clearSessionPlanComments: vi.fn(),
|
||||
handleClearPRFeedback: vi.fn(),
|
||||
handleClearWalkthroughComments: vi.fn(),
|
||||
clearEphemeral: vi.fn(),
|
||||
addContextFile: vi.fn(),
|
||||
planModeEnabled: false,
|
||||
...overrides,
|
||||
} as never;
|
||||
}
|
||||
function panelState(overrides = {}) {
|
||||
return {
|
||||
resolvedSessionId: "session-1",
|
||||
taskId: "task-1",
|
||||
sessionModel: null,
|
||||
activeModel: null,
|
||||
isAgentBusy: false,
|
||||
activeDocument: null,
|
||||
planComments: [],
|
||||
pendingPRFeedback: [],
|
||||
walkthroughComments: [],
|
||||
messageComments: [],
|
||||
contextFiles: [],
|
||||
prompts: [],
|
||||
markCommentsSent: vi.fn(),
|
||||
clearSessionPlanComments: vi.fn(),
|
||||
handleClearPRFeedback: vi.fn(),
|
||||
handleClearWalkthroughComments: vi.fn(),
|
||||
clearEphemeral: vi.fn(),
|
||||
addContextFile: vi.fn(),
|
||||
planModeEnabled: false,
|
||||
...overrides,
|
||||
} as never;
|
||||
}
|
||||
|
||||
describe("resolveInputPlaceholder", () => {
|
||||
it("invites queueing while a clarification remains pending", () => {
|
||||
expect(resolveInputPlaceholder(false, undefined, false, true, false)).toBe(
|
||||
"Queue instructions while the question is pending...",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("useSubmitHandler", () => {
|
||||
it("shows a toast when sending fails", async () => {
|
||||
vi.spyOn(console, "error").mockImplementation(() => undefined);
|
||||
handleSendMessageMock.mockRejectedValueOnce(new Error("WebSocket request timed out"));
|
||||
@@ -125,6 +134,32 @@ describe("useSubmitHandler", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("queues through the shared handler instead of preview onSend while clarification is pending", async () => {
|
||||
const onSend = vi.fn();
|
||||
const { result } = renderHook(() =>
|
||||
useSubmitHandler(panelState({ pendingClarification: { id: "clarification-1" } }), onSend),
|
||||
);
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleSubmit({ message: "queued instruction" });
|
||||
});
|
||||
|
||||
expect(onSend).not.toHaveBeenCalled();
|
||||
expect(handleSendMessageMock).toHaveBeenCalledWith({ message: "queued instruction" });
|
||||
});
|
||||
|
||||
it("uses preview onSend when no clarification is pending", async () => {
|
||||
const onSend = vi.fn();
|
||||
const { result } = renderHook(() => useSubmitHandler(panelState(), onSend));
|
||||
|
||||
await act(async () => {
|
||||
await result.current.handleSubmit({ message: "direct preview message" });
|
||||
});
|
||||
|
||||
expect(onSend).toHaveBeenCalledWith({ message: "direct preview message" });
|
||||
expect(handleSendMessageMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("reports deterministic preflight failures as not sent", async () => {
|
||||
vi.spyOn(console, "error").mockImplementation(() => undefined);
|
||||
const error = Object.assign(new Error("Connection unavailable. Reconnect and try again."), {
|
||||
|
||||
@@ -78,7 +78,7 @@ export function buildSubmitMessage({
|
||||
return finalMessage;
|
||||
}
|
||||
|
||||
function resolveInputPlaceholder(
|
||||
export function resolveInputPlaceholder(
|
||||
isAgentBusy: boolean,
|
||||
activeDocumentType: string | undefined,
|
||||
planModeEnabled: boolean,
|
||||
@@ -86,7 +86,7 @@ function resolveInputPlaceholder(
|
||||
needsRecovery: boolean,
|
||||
): string {
|
||||
if (needsRecovery) return "Choose a recovery option above to continue...";
|
||||
if (hasClarification) return "Answer the question above to continue...";
|
||||
if (hasClarification) return "Queue instructions while the question is pending...";
|
||||
if (isAgentBusy) return "Queue instructions to the agent...";
|
||||
if (activeDocumentType === "file") return "Continue working on the file...";
|
||||
if (planModeEnabled) return "Continue working on the plan...";
|
||||
@@ -141,6 +141,7 @@ function usePanelMessageHandler(panelState: ReturnType<typeof useChatPanelState>
|
||||
sessionModel,
|
||||
activeModel,
|
||||
isAgentBusy,
|
||||
pendingClarification,
|
||||
activeDocument,
|
||||
planComments,
|
||||
contextFiles,
|
||||
@@ -153,6 +154,7 @@ function usePanelMessageHandler(panelState: ReturnType<typeof useChatPanelState>
|
||||
activeModel,
|
||||
planModeEnabled: panelState.planModeEnabled,
|
||||
isAgentBusy,
|
||||
hasPendingClarification: !!pendingClarification,
|
||||
activeDocument,
|
||||
planComments,
|
||||
contextFiles,
|
||||
@@ -180,6 +182,7 @@ export function useSubmitHandler(
|
||||
clearEphemeral,
|
||||
addContextFile,
|
||||
planModeEnabled,
|
||||
pendingClarification,
|
||||
} = panelState;
|
||||
const { handleSendMessage } = usePanelMessageHandler(panelState);
|
||||
|
||||
@@ -197,7 +200,7 @@ export function useSubmitHandler(
|
||||
messageComments,
|
||||
});
|
||||
const outbound = { ...payload, message: finalMessage };
|
||||
if (onSend) {
|
||||
if (onSend && !pendingClarification) {
|
||||
// Expand task mentions because onSend bypasses useMessageHandler.buildFinalMessage.
|
||||
const taskCtx = payload.inlineTaskMentions?.length
|
||||
? buildTaskMentionsContext(payload.inlineTaskMentions, storeApi.getState())
|
||||
@@ -229,6 +232,7 @@ export function useSubmitHandler(
|
||||
[
|
||||
isSending,
|
||||
onSend,
|
||||
pendingClarification,
|
||||
storeApi,
|
||||
handleSendMessage,
|
||||
markCommentsSent,
|
||||
@@ -431,6 +435,10 @@ function useChatInputDerived(
|
||||
return { planActions, executor, placeholder };
|
||||
}
|
||||
|
||||
function canManuallyDrainQueue(pendingClarification: unknown, sessionState: string | null) {
|
||||
return !pendingClarification && (sessionState === "WAITING_FOR_INPUT" || sessionState === "IDLE");
|
||||
}
|
||||
|
||||
export function ChatInputArea({
|
||||
chatInputRef,
|
||||
clarificationKey,
|
||||
@@ -450,7 +458,7 @@ export function ChatInputArea({
|
||||
const { resolvedSessionId, taskId, isAgentBusy, needsRecovery, planModeEnabled } = panelState;
|
||||
const composerWorkspaceId = useComposerWorkspaceId(resolvedSessionId, taskId);
|
||||
const sessionState = panelState.session?.state ?? null;
|
||||
const canDrainQueue = sessionState === "WAITING_FOR_INPUT" || sessionState === "IDLE";
|
||||
const canDrainQueue = canManuallyDrainQueue(panelState.pendingClarification, sessionState);
|
||||
const { planActions, executor, placeholder } = useChatInputDerived(
|
||||
panelState,
|
||||
chatInputRef,
|
||||
|
||||
@@ -3,6 +3,7 @@ import { cleanup, render, screen } from "@testing-library/react";
|
||||
import { TooltipProvider } from "@kandev/ui/tooltip";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { ChatInputBody, type ChatInputBodyProps } from "./chat-input-body";
|
||||
import { shouldShowCancelAgent } from "./chat-input-container";
|
||||
|
||||
const tipTapPropsMock = vi.hoisted(() => vi.fn());
|
||||
|
||||
@@ -53,7 +54,6 @@ function props(overrides: Partial<ChatInputBodyProps> = {}): ChatInputBodyProps
|
||||
inputPlaceholder: "Ask to make changes",
|
||||
isDisabled: false,
|
||||
submitDisabled: false,
|
||||
hasClarification: false,
|
||||
planModeEnabled: false,
|
||||
planModeAvailable: true,
|
||||
mcpServers: [],
|
||||
@@ -80,6 +80,20 @@ function props(overrides: Partial<ChatInputBodyProps> = {}): ChatInputBodyProps
|
||||
}
|
||||
|
||||
describe("ChatInputBody", () => {
|
||||
it("keeps the regular editor enabled while a structured clarification is pending", () => {
|
||||
render(
|
||||
<TooltipProvider>
|
||||
<ChatInputBody
|
||||
{...props({
|
||||
hasClarification: true,
|
||||
})}
|
||||
/>
|
||||
</TooltipProvider>,
|
||||
);
|
||||
|
||||
expect(tipTapPropsMock).toHaveBeenCalledWith(expect.objectContaining({ disabled: false }));
|
||||
});
|
||||
|
||||
it("keeps entity references explicitly disabled unless the chat surface enables them", () => {
|
||||
render(
|
||||
<TooltipProvider>
|
||||
@@ -115,3 +129,32 @@ describe("ChatInputBody", () => {
|
||||
expect(screen.getByTestId("mock-tiptap-input").parentElement?.className).not.toContain("pr-28");
|
||||
});
|
||||
});
|
||||
|
||||
describe("shouldShowCancelAgent", () => {
|
||||
const detachedClarification = { metadata: { agent_disconnected: true } } as never;
|
||||
const attachedClarification = {} as never;
|
||||
|
||||
it.each([
|
||||
[
|
||||
"suppresses cancel for detached clarification while session is RUNNING",
|
||||
true,
|
||||
detachedClarification,
|
||||
false,
|
||||
],
|
||||
[
|
||||
"suppresses cancel for detached clarification while session is WAITING",
|
||||
false,
|
||||
detachedClarification,
|
||||
false,
|
||||
],
|
||||
[
|
||||
"keeps cancel for clarification without detached metadata",
|
||||
false,
|
||||
attachedClarification,
|
||||
true,
|
||||
],
|
||||
["keeps cancel for a running session without clarification", true, null, true],
|
||||
])("%s", (_name, isAgentBusy, pendingClarification, expected) => {
|
||||
expect(shouldShowCancelAgent(isAgentBusy, pendingClarification)).toBe(expected);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -20,7 +20,6 @@ export type ChatInputEditorAreaProps = {
|
||||
isDisabled: boolean;
|
||||
submitDisabled: boolean;
|
||||
submitDisabledReason?: string;
|
||||
hasClarification: boolean;
|
||||
planModeEnabled: boolean;
|
||||
planModeAvailable: boolean;
|
||||
mcpServers: string[];
|
||||
@@ -41,6 +40,7 @@ export type ChatInputEditorAreaProps = {
|
||||
hideAgentControls?: boolean;
|
||||
hidePlanMode?: boolean;
|
||||
isAgentBusy: boolean;
|
||||
canCancelAgent?: boolean;
|
||||
onPlanModeChange: (enabled: boolean) => void;
|
||||
taskTitle?: string;
|
||||
taskDescription: string;
|
||||
@@ -125,7 +125,7 @@ function FileInput({
|
||||
|
||||
export function ChatInputEditorArea(p: ChatInputEditorAreaProps) {
|
||||
const { inputRef, value, handleChange, handleSubmitWithReset, inputPlaceholder } = p;
|
||||
const { isDisabled, hasClarification, planModeEnabled, planModeAvailable, mcpServers } = p;
|
||||
const { isDisabled, planModeEnabled, planModeAvailable, mcpServers } = p;
|
||||
const { submitKey, setIsInputFocused, sessionId, taskId, planContextEnabled } = p;
|
||||
const { onAddContextFile, onToggleContextFile, addFiles, fileInputRef } = p;
|
||||
const { showRequestChangesTooltip, isAgentBusy, onPlanModeChange, taskTitle, taskDescription } =
|
||||
@@ -155,7 +155,7 @@ export function ChatInputEditorArea(p: ChatInputEditorAreaProps) {
|
||||
onChange={handleChange}
|
||||
onSubmit={wrappedSubmit}
|
||||
placeholder={inputPlaceholder}
|
||||
disabled={isDisabled || hasClarification}
|
||||
disabled={isDisabled}
|
||||
planModeEnabled={planModeEnabled}
|
||||
submitKey={submitKey}
|
||||
onFocus={() => setIsInputFocused(true)}
|
||||
@@ -182,6 +182,7 @@ export function ChatInputEditorArea(p: ChatInputEditorAreaProps) {
|
||||
taskTitle={taskTitle}
|
||||
taskDescription={taskDescription}
|
||||
isAgentBusy={isAgentBusy}
|
||||
canCancelAgent={p.canCancelAgent}
|
||||
hasContent={hasContent}
|
||||
isDisabled={p.submitDisabled}
|
||||
submitDisabledReason={p.submitDisabledReason}
|
||||
|
||||
@@ -7,7 +7,7 @@ import { Tooltip, TooltipContent, TooltipTrigger } from "@kandev/ui/tooltip";
|
||||
import { NewSessionDialog } from "@/components/task/new-session-dialog";
|
||||
import { useAppStore } from "@/components/state-provider";
|
||||
import type { ContextFile } from "@/lib/state/context-files-store";
|
||||
import type { Message } from "@/lib/types/http";
|
||||
import type { ClarificationRequestMetadata, Message } from "@/lib/types/http";
|
||||
import type { DiffComment } from "@/lib/diff/types";
|
||||
import type { TaskMentionData } from "@/hooks/use-inline-mention";
|
||||
import type { EntityReference } from "@/lib/types/entity-reference";
|
||||
@@ -269,6 +269,15 @@ type EnhancePromptExtras = {
|
||||
onVoiceAutoSend?: () => void;
|
||||
};
|
||||
|
||||
export function shouldShowCancelAgent(
|
||||
isAgentBusy: boolean,
|
||||
pendingClarification: Message | null | undefined,
|
||||
): boolean {
|
||||
if (!pendingClarification) return isAgentBusy;
|
||||
return !(pendingClarification.metadata as ClarificationRequestMetadata | undefined)
|
||||
?.agent_disconnected;
|
||||
}
|
||||
|
||||
function buildEditorAreaProps(
|
||||
s: ContainerState,
|
||||
p: NormalizedChatInputProps,
|
||||
@@ -283,7 +292,6 @@ function buildEditorAreaProps(
|
||||
isDisabled: s.isDisabled,
|
||||
submitDisabled: s.submitDisabled,
|
||||
submitDisabledReason: s.submitDisabledReason,
|
||||
hasClarification: s.hasClarification,
|
||||
planModeEnabled: p.planModeEnabled,
|
||||
planModeAvailable: p.planModeAvailable ?? true,
|
||||
mcpServers: p.mcpServers ?? [],
|
||||
@@ -299,7 +307,8 @@ function buildEditorAreaProps(
|
||||
addFiles: s.addFiles,
|
||||
fileInputRef: s.fileInputRef,
|
||||
showRequestChangesTooltip: p.showRequestChangesTooltip,
|
||||
isAgentBusy: p.isAgentBusy,
|
||||
isAgentBusy: p.isAgentBusy || !!(p.pendingClarification && p.onClarificationResolved),
|
||||
canCancelAgent: shouldShowCancelAgent(p.isAgentBusy, p.pendingClarification),
|
||||
onPlanModeChange: p.onPlanModeChange,
|
||||
taskTitle: p.taskTitle,
|
||||
taskDescription: p.taskDescription,
|
||||
|
||||
@@ -141,6 +141,7 @@ function DesktopRightSection(props: {
|
||||
hideAgentControls: boolean;
|
||||
planModeEnabled: boolean;
|
||||
isAgentBusy: boolean;
|
||||
canCancelAgent?: boolean;
|
||||
hasContent: boolean;
|
||||
onImplementPlan?: (fresh: boolean) => void;
|
||||
isDisabled: boolean;
|
||||
@@ -178,6 +179,7 @@ function DesktopRightSection(props: {
|
||||
)}
|
||||
<SubmitButton
|
||||
isAgentBusy={props.isAgentBusy}
|
||||
canCancelAgent={props.canCancelAgent}
|
||||
hasContent={props.hasContent}
|
||||
isDisabled={props.isDisabled}
|
||||
submitDisabledReason={props.submitDisabledReason}
|
||||
@@ -261,6 +263,7 @@ export function DesktopChatInputToolbar(props: DesktopToolbarProps) {
|
||||
hideAgentControls={props.hideAgentControls}
|
||||
planModeEnabled={props.planModeEnabled}
|
||||
isAgentBusy={props.isAgentBusy}
|
||||
canCancelAgent={props.canCancelAgent}
|
||||
hasContent={props.hasContent ?? false}
|
||||
onImplementPlan={props.onImplementPlan}
|
||||
isDisabled={props.isDisabled}
|
||||
|
||||
@@ -40,6 +40,7 @@ type MobileToolbarProps = {
|
||||
contextFiles: ContextFile[];
|
||||
onToggleFile: (file: ContextFile) => void;
|
||||
isAgentBusy: boolean;
|
||||
canCancelAgent?: boolean;
|
||||
hasContent: boolean;
|
||||
onImplementPlan?: (fresh: boolean) => void;
|
||||
onEnhancePrompt?: () => void;
|
||||
@@ -181,6 +182,7 @@ export function MobileChatInputToolbar(props: MobileToolbarProps) {
|
||||
)}
|
||||
<SubmitButton
|
||||
isAgentBusy={props.isAgentBusy}
|
||||
canCancelAgent={props.canCancelAgent}
|
||||
hasContent={props.hasContent}
|
||||
isDisabled={props.isDisabled}
|
||||
submitDisabledReason={props.submitDisabledReason}
|
||||
|
||||
@@ -22,6 +22,7 @@ import { cn } from "@/lib/utils";
|
||||
|
||||
type SubmitButtonProps = {
|
||||
isAgentBusy: boolean;
|
||||
canCancelAgent?: boolean;
|
||||
hasContent: boolean;
|
||||
isDisabled: boolean;
|
||||
submitDisabledReason?: string;
|
||||
@@ -92,6 +93,7 @@ function SendSubmitButton({
|
||||
|
||||
export function SubmitButton({
|
||||
isAgentBusy,
|
||||
canCancelAgent = isAgentBusy,
|
||||
hasContent,
|
||||
isDisabled,
|
||||
submitDisabledReason,
|
||||
@@ -125,7 +127,7 @@ export function SubmitButton({
|
||||
|
||||
return (
|
||||
<div className="flex items-center gap-1">
|
||||
{isAgentBusy && (
|
||||
{canCancelAgent && (
|
||||
<Tooltip>
|
||||
<TooltipTrigger asChild>
|
||||
<Button
|
||||
|
||||
@@ -159,6 +159,14 @@ function renderFullToolbar(overrides: Partial<ChatInputToolbarProps> = {}) {
|
||||
// tears down a long-running tool (Claude Monitor, etc.) sends N cancel requests
|
||||
// to the backend, each producing a duplicate "Turn cancelled by user" message.
|
||||
describe("ChatInputToolbar cancel button", () => {
|
||||
it("keeps the queue affordance without a cancel control after clarification detaches", () => {
|
||||
renderFullToolbar({ isAgentBusy: true, canCancelAgent: false });
|
||||
|
||||
expect(screen.queryByTestId("cancel-agent-button")).toBeNull();
|
||||
expect(screen.getByTestId("submit-message-button")).toBeTruthy();
|
||||
expect(screen.getByText("Queue message")).toBeTruthy();
|
||||
});
|
||||
|
||||
it("disables itself and blocks duplicate clicks while cancel is in flight", async () => {
|
||||
const { promise, resolve } = deferred<void>();
|
||||
const onCancel = vi.fn(() => promise);
|
||||
|
||||
@@ -20,6 +20,7 @@ export type ChatInputToolbarProps = {
|
||||
taskTitle?: string;
|
||||
taskDescription: string;
|
||||
isAgentBusy: boolean;
|
||||
canCancelAgent?: boolean;
|
||||
/** Whether the input has content to send (text, comments, or context) */
|
||||
hasContent?: boolean;
|
||||
isDisabled: boolean;
|
||||
@@ -62,6 +63,7 @@ export type ChatInputToolbarProps = {
|
||||
|
||||
function MinimalToolbar({
|
||||
isAgentBusy,
|
||||
canCancelAgent,
|
||||
hasContent,
|
||||
isDisabled,
|
||||
submitDisabledReason,
|
||||
@@ -72,6 +74,7 @@ function MinimalToolbar({
|
||||
}: Pick<
|
||||
ChatInputToolbarProps,
|
||||
| "isAgentBusy"
|
||||
| "canCancelAgent"
|
||||
| "hasContent"
|
||||
| "isDisabled"
|
||||
| "submitDisabledReason"
|
||||
@@ -85,6 +88,7 @@ function MinimalToolbar({
|
||||
<div className="flex items-center justify-end gap-1 px-1 pt-0 pb-0.5 border-t border-border">
|
||||
<SubmitButton
|
||||
isAgentBusy={isAgentBusy}
|
||||
canCancelAgent={canCancelAgent}
|
||||
hasContent={hasContent ?? false}
|
||||
isDisabled={isDisabled}
|
||||
submitDisabledReason={submitDisabledReason}
|
||||
@@ -122,6 +126,7 @@ export const ChatInputToolbar = memo(function ChatInputToolbar(rawProps: ChatInp
|
||||
return (
|
||||
<MinimalToolbar
|
||||
isAgentBusy={props.isAgentBusy}
|
||||
canCancelAgent={props.canCancelAgent}
|
||||
hasContent={props.hasContent}
|
||||
isDisabled={props.isDisabled}
|
||||
submitDisabledReason={props.submitDisabledReason}
|
||||
@@ -156,6 +161,7 @@ export const ChatInputToolbar = memo(function ChatInputToolbar(rawProps: ChatInp
|
||||
contextFiles={props.contextFiles}
|
||||
onToggleFile={props.onToggleFile ?? (() => {})}
|
||||
isAgentBusy={props.isAgentBusy}
|
||||
canCancelAgent={props.canCancelAgent}
|
||||
hasContent={props.hasContent ?? false}
|
||||
onImplementPlan={props.onImplementPlan}
|
||||
onEnhancePrompt={props.onEnhancePrompt}
|
||||
|
||||
@@ -184,17 +184,19 @@ type CarouselShortcutArgs = {
|
||||
onSubmit: () => void;
|
||||
};
|
||||
|
||||
function isEditableTarget(target: EventTarget | null): target is HTMLElement {
|
||||
return (
|
||||
target instanceof HTMLElement &&
|
||||
(target.isContentEditable || target.tagName === "INPUT" || target.tagName === "TEXTAREA")
|
||||
);
|
||||
}
|
||||
|
||||
// shouldIgnoreShortcut filters out events that the overlay must not handle:
|
||||
// keystrokes inside an input/textarea (the user is typing) and any modifier
|
||||
// keystrokes inside an editable control (the user is typing) and any modifier
|
||||
// combo (so we don't hijack browser shortcuts like Cmd/Ctrl+1..9 for tab
|
||||
// switching or Alt+ArrowLeft for back-navigation).
|
||||
function shouldIgnoreShortcut(e: KeyboardEvent): boolean {
|
||||
if (
|
||||
e.target instanceof HTMLElement &&
|
||||
(e.target.tagName === "INPUT" || e.target.tagName === "TEXTAREA")
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
if (isEditableTarget(e.target)) return true;
|
||||
return e.metaKey || e.ctrlKey || e.altKey || e.shiftKey;
|
||||
}
|
||||
|
||||
@@ -205,10 +207,7 @@ function shouldIgnoreShortcut(e: KeyboardEvent): boolean {
|
||||
function tryHandleMetaEnter(e: KeyboardEvent, canSubmit: boolean, onSubmit: () => void): boolean {
|
||||
if (e.key !== "Enter" || e.shiftKey || e.altKey) return false;
|
||||
if (!e.metaKey && !e.ctrlKey) return false;
|
||||
const inEditable =
|
||||
e.target instanceof HTMLElement &&
|
||||
(e.target.tagName === "INPUT" || e.target.tagName === "TEXTAREA");
|
||||
if (inEditable) return true;
|
||||
if (isEditableTarget(e.target)) return true;
|
||||
e.preventDefault();
|
||||
if (canSubmit) onSubmit();
|
||||
return true;
|
||||
|
||||
@@ -54,6 +54,23 @@ describe("useChatInputContainer", () => {
|
||||
|
||||
expect(result.current.submitDisabledReason).toBe("The agent is still being set up.");
|
||||
});
|
||||
|
||||
it("keeps queueing enabled for an interactive clarification during stale startup", () => {
|
||||
const { result } = renderInputState({
|
||||
isStarting: true,
|
||||
isPreparingEnvironment: true,
|
||||
placeholder: "Queue instructions while the question is pending...",
|
||||
pendingClarification: { id: "clarification-1" } as never,
|
||||
onClarificationResolved: vi.fn(),
|
||||
});
|
||||
|
||||
expect(result.current.isDisabled).toBe(false);
|
||||
expect(result.current.submitDisabled).toBe(false);
|
||||
expect(result.current.submitDisabledReason).toBeUndefined();
|
||||
expect(result.current.inputPlaceholder).toBe(
|
||||
"Queue instructions while the question is pending...",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("shouldShowChatFocusHint", () => {
|
||||
|
||||
@@ -121,13 +121,12 @@ function computeDerivedState(params: {
|
||||
isAgentBusy: boolean;
|
||||
hasAgentCommands: boolean;
|
||||
}) {
|
||||
// STARTING is included so the editor itself stays uneditable until the
|
||||
// session reaches RUNNING. Without this, the e2e suite's wait-for-
|
||||
// contenteditable would fire mid-STARTING, the test would press Cmd+Enter,
|
||||
// and the backend would reject with "Failed to send message to agent"
|
||||
// before the agent process is ready to receive prompts.
|
||||
const hasClarification = !!(params.pendingClarification && params.onClarificationResolved);
|
||||
// STARTING blocks regular messages until the session reaches RUNNING. An
|
||||
// interactive clarification is different: its queue path is persistence-only,
|
||||
// so it remains safe while stale lifecycle metadata says STARTING.
|
||||
const isDisabled =
|
||||
params.isStarting ||
|
||||
(params.isStarting && !hasClarification) ||
|
||||
params.isMoving ||
|
||||
params.isSending ||
|
||||
params.isFailed ||
|
||||
@@ -138,10 +137,8 @@ function computeDerivedState(params: {
|
||||
// container/sandbox is actively bootstrapping. The brief STARTING
|
||||
// transition for local quick-chat sessions doesn't deserve its own
|
||||
// tooltip — the editor is disabled, that's the signal.
|
||||
const submitDisabledReason = params.isPreparingEnvironment
|
||||
? "The agent is still being set up."
|
||||
: undefined;
|
||||
const hasClarification = !!(params.pendingClarification && params.onClarificationResolved);
|
||||
const submitDisabledReason =
|
||||
isDisabled && params.isPreparingEnvironment ? "The agent is still being set up." : undefined;
|
||||
const hasPendingComments = !!(
|
||||
params.pendingCommentsByFile && Object.keys(params.pendingCommentsByFile).length > 0
|
||||
);
|
||||
@@ -156,7 +153,7 @@ function computeDerivedState(params: {
|
||||
params.placeholder,
|
||||
params.isAgentBusy,
|
||||
params.hasAgentCommands,
|
||||
params.isStarting,
|
||||
params.isStarting && !hasClarification,
|
||||
);
|
||||
return {
|
||||
isDisabled,
|
||||
|
||||
@@ -33,6 +33,36 @@ useRegularMode();
|
||||
test.describe("Clarification flow", () => {
|
||||
test.describe.configure({ retries: 1 });
|
||||
|
||||
test("keeps a pending question open while typing a digit in the regular composer", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
const session = await seedClarificationTask(
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
"Clarification Composer Queue",
|
||||
"clarification",
|
||||
);
|
||||
|
||||
await expect(session.clarificationOverlay()).toBeVisible({ timeout: 30_000 });
|
||||
const composer = session.activeChat().getByTestId("chat-input-editor");
|
||||
await expect(composer).toHaveAttribute("contenteditable", "true", { timeout: 30_000 });
|
||||
await expect(testPage.getByTestId("cancel-agent-button")).toBeVisible();
|
||||
|
||||
await composer.pressSequentially("Queue this after I answer 1", { timeout: 30_000 });
|
||||
await expect(composer).toContainText("Queue this after I answer 1");
|
||||
await expect(session.clarificationOverlay()).toBeVisible();
|
||||
await testPage.getByTestId("submit-message-button").click();
|
||||
|
||||
await expect(testPage.getByTestId("queue-chip")).toBeVisible({ timeout: 10_000 });
|
||||
await expect(session.clarificationOverlay()).toBeVisible();
|
||||
await testPage.getByTestId("queue-chip").click();
|
||||
await expect(testPage.getByTestId("queued-ghost-list")).toBeVisible();
|
||||
await expect(testPage.getByTestId("queue-drain-next")).not.toBeVisible();
|
||||
});
|
||||
|
||||
test("select option (happy path)", async ({ testPage, apiClient, seedData }) => {
|
||||
const session = await seedClarificationTask(
|
||||
testPage,
|
||||
|
||||
@@ -10,6 +10,34 @@ import { seedClarificationSession } from "../../helpers/clarification";
|
||||
test.describe("Mobile clarification multiline answer", () => {
|
||||
test.describe.configure({ retries: 1, timeout: 120_000 });
|
||||
|
||||
test("keeps a pending question open while typing a digit in the inline composer", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
}) => {
|
||||
const session = await seedClarificationSession(
|
||||
testPage,
|
||||
apiClient,
|
||||
seedData,
|
||||
"Mobile Clarify Composer Queue",
|
||||
{ scenario: "clarification" },
|
||||
);
|
||||
|
||||
await expect(session.clarificationOverlay()).toBeVisible({ timeout: 30_000 });
|
||||
const composer = session.activeChat().getByTestId("chat-input-editor");
|
||||
await expect(composer).toHaveAttribute("contenteditable", "true", { timeout: 30_000 });
|
||||
await composer.pressSequentially("Queue this from phone 1", { timeout: 30_000 });
|
||||
await expect(composer).toContainText("Queue this from phone 1");
|
||||
await expect(session.clarificationOverlay()).toBeVisible();
|
||||
await testPage.getByTestId("submit-message-button").tap();
|
||||
|
||||
await expect(testPage.getByTestId("queue-chip")).toBeVisible({ timeout: 10_000 });
|
||||
await expect(session.clarificationOverlay()).toBeVisible();
|
||||
await testPage.getByTestId("queue-chip").tap();
|
||||
await expect(testPage.getByTestId("queued-ghost-list")).toBeVisible();
|
||||
await expect(testPage.getByTestId("queue-drain-next")).not.toBeVisible();
|
||||
});
|
||||
|
||||
test("Enter inserts a newline and the Send button submits the multiline answer", async ({
|
||||
testPage,
|
||||
apiClient,
|
||||
|
||||
@@ -1,19 +1,30 @@
|
||||
import { renderHook } from "@testing-library/react";
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import {
|
||||
buildContextFilesContext,
|
||||
buildTaskMentionsContext,
|
||||
sendMessageRequest,
|
||||
useMessageHandler,
|
||||
} from "./use-message-handler";
|
||||
import type { AppState } from "@/lib/state/store";
|
||||
import type { TaskMentionData } from "./use-inline-mention";
|
||||
import type { EntityReference } from "@/lib/types/entity-reference";
|
||||
|
||||
const getWebSocketClientMock = vi.hoisted(() => vi.fn());
|
||||
const queueMock = vi.hoisted(() => vi.fn().mockResolvedValue(undefined));
|
||||
|
||||
vi.mock("@/lib/ws/connection", () => ({
|
||||
getWebSocketClient: getWebSocketClientMock,
|
||||
}));
|
||||
|
||||
vi.mock("@/hooks/domains/session/use-queue", () => ({
|
||||
useQueue: () => ({ queue: queueMock }),
|
||||
}));
|
||||
|
||||
vi.mock("@/components/state-provider", () => ({
|
||||
useAppStoreApi: () => ({ getState: () => ({}) }),
|
||||
}));
|
||||
|
||||
const IMPROVE_HARNESS_PROMPT = "improve-harness";
|
||||
const IMPROVE_HARNESS_CONTENT = "Review this session for durable harness improvements.";
|
||||
|
||||
@@ -252,3 +263,26 @@ describe("sendMessageRequest", () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("useMessageHandler", () => {
|
||||
it("queues regular composer input while a clarification is pending", async () => {
|
||||
const request = vi.fn();
|
||||
getWebSocketClientMock.mockReturnValue({ request });
|
||||
const { result } = renderHook(() =>
|
||||
useMessageHandler({
|
||||
resolvedSessionId: "session-1",
|
||||
taskId: "task-1",
|
||||
sessionModel: null,
|
||||
activeModel: null,
|
||||
hasPendingClarification: true,
|
||||
}),
|
||||
);
|
||||
|
||||
await result.current.handleSendMessage({ message: "Queue this after I answer" });
|
||||
|
||||
expect(queueMock).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ content: "Queue this after I answer", taskId: "task-1" }),
|
||||
);
|
||||
expect(request).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -143,6 +143,7 @@ export interface UseMessageHandlerParams {
|
||||
activeModel: string | null;
|
||||
planModeEnabled?: boolean;
|
||||
isAgentBusy?: boolean;
|
||||
hasPendingClarification?: boolean;
|
||||
activeDocument?: ActiveDocument | null;
|
||||
planComments?: PlanComment[];
|
||||
contextFiles?: ContextFile[];
|
||||
@@ -209,6 +210,7 @@ export function useMessageHandler({
|
||||
activeModel,
|
||||
planModeEnabled = false,
|
||||
isAgentBusy = false,
|
||||
hasPendingClarification = false,
|
||||
activeDocument = null,
|
||||
planComments = [],
|
||||
contextFiles = [],
|
||||
@@ -256,7 +258,7 @@ export function useMessageHandler({
|
||||
const contextFilesMeta =
|
||||
realFiles.length > 0 ? realFiles.map((f) => ({ path: f.path, name: f.name })) : undefined;
|
||||
|
||||
if (isAgentBusy) {
|
||||
if (isAgentBusy || hasPendingClarification) {
|
||||
const queueAttachments = payload.attachments?.map((att) => ({
|
||||
type: att.type,
|
||||
data: att.data,
|
||||
@@ -300,6 +302,7 @@ export function useMessageHandler({
|
||||
sessionModel,
|
||||
planModeEnabled,
|
||||
isAgentBusy,
|
||||
hasPendingClarification,
|
||||
queue,
|
||||
buildFinalMessage,
|
||||
storeApi,
|
||||
|
||||
Reference in New Issue
Block a user