diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.test.tsx b/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.test.tsx index b3bcaaa463e3..dd83d04f0c0f 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.test.tsx +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.test.tsx @@ -1,7 +1,9 @@ import { act, cleanup, renderHook, waitFor } from '@testing-library/react' import { afterEach, describe, expect, it, vi } from 'vitest' +import { $clarifyRequests } from '@/store/clarify' import type { ComposerAttachment } from '@/store/composer' +import { $gateway } from '@/store/gateway' import { useComposerSubmit } from './use-composer-submit' @@ -184,3 +186,80 @@ describe('useComposerSubmit busy-turn routing', () => { ) }) }) + +describe('useComposerSubmit with a clarify parked on the session', () => { + const gatewayRequest = vi.fn(async () => ({ ok: true })) + + const parkClarify = (sessionId: string) => { + $clarifyRequests.set({ + [sessionId]: { requestId: `req-${sessionId}`, question: 'which one?', choices: ['a', 'b'], sessionId } + }) + $gateway.set({ request: gatewayRequest } as unknown as ReturnType) + } + + afterEach(() => { + cleanup() + gatewayRequest.mockClear() + $clarifyRequests.set({}) + $gateway.set(null) + vi.restoreAllMocks() + }) + + it('skips the question and still sends the typed message on an idle session', async () => { + parkClarify('runtime-session') + const { hook, onSubmit } = renderSubmitHook({ text: 'actually do this instead' }) + + act(() => { + hook.result.current.submitDraft() + }) + + await waitFor(() => + expect(gatewayRequest).toHaveBeenCalledWith('clarify.respond', { + request_id: 'req-runtime-session', + answer: '' + }) + ) + await waitFor(() => + expect(onSubmit).toHaveBeenCalledWith('actually do this instead', expect.objectContaining({ attachments: [] })) + ) + expect($clarifyRequests.get()['runtime-session']).toBeUndefined() + }) + + it('skips the question before steering a busy turn', async () => { + parkClarify('runtime-session') + const { hook, onSteer } = renderSubmitHook({ busy: true, text: 'change course' }) + + act(() => { + hook.result.current.submitDraft() + }) + + await waitFor(() => expect(onSteer).toHaveBeenCalledWith('change course')) + expect(gatewayRequest).toHaveBeenCalledWith('clarify.respond', { request_id: 'req-runtime-session', answer: '' }) + }) + + it('leaves the question alone for an empty Enter (Stop, not an answer)', () => { + parkClarify('runtime-session') + const { hook, onCancel } = renderSubmitHook({ busy: true }) + + act(() => { + hook.result.current.submitDraft() + }) + + expect(gatewayRequest).not.toHaveBeenCalled() + expect($clarifyRequests.get()['runtime-session']).toBeDefined() + expect(onCancel).toHaveBeenCalledTimes(1) + }) + + it("leaves another session's question alone", async () => { + parkClarify('other-session') + const { hook, onSubmit } = renderSubmitHook({ text: 'unrelated message' }) + + act(() => { + hook.result.current.submitDraft() + }) + + await waitFor(() => expect(onSubmit).toHaveBeenCalled()) + expect(gatewayRequest).not.toHaveBeenCalled() + expect($clarifyRequests.get()['other-session']).toBeDefined() + }) +}) diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.ts b/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.ts index 315157a96e55..fff0b8153fcd 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-submit.ts @@ -2,6 +2,7 @@ import { type RefObject, useEffect, useRef } from 'react' import { SLASH_COMMAND_RE } from '@/lib/chat-runtime' import { triggerHaptic } from '@/lib/haptics' +import { hasClarifyRequest, skipClarifyRequest } from '@/store/clarify' import { clearSessionDraft, type ComposerAttachment } from '@/store/composer' import { resetBrowseState } from '@/store/composer-input-history' import { enqueueQueuedPrompt, type QueuedPromptEntry } from '@/store/composer-queue' @@ -141,6 +142,21 @@ export function useComposerSubmit({ const text = draftRef.current const payloadPresent = text.trim().length > 0 || attachments.length > 0 + // A clarify card parked on this session owns the turn: the agent is blocked + // inside its tool batch waiting on `clarify.respond`, so a follow-up routed + // through steer/queue sits undelivered until the clarify's own timeout + // (default 5 min) — the message looks sent and nothing happens. Typing a + // real message instead of picking an option IS the answer "none of these": + // skip the question so the tool returns, then route the words normally. + // + // Fire-and-forget, not awaited: the skip clears the card synchronously and + // both RPCs ride the same socket in call order, so the gateway resolves the + // clarify before it sees the follow-up. Awaiting first would leave the draft + // live for a tick — long enough for a second Enter to send it twice. + if (payloadPresent && !queueEdit && hasClarifyRequest(sessionId)) { + void skipClarifyRequest(sessionId) + } + if (queueEdit) { exitQueuedEdit('save') } else if (busy) { diff --git a/apps/desktop/src/components/assistant-ui/clarify-tool.test.tsx b/apps/desktop/src/components/assistant-ui/clarify-tool.test.tsx index 7e0daf7ef34a..ecb4c6c2faaa 100644 --- a/apps/desktop/src/components/assistant-ui/clarify-tool.test.tsx +++ b/apps/desktop/src/components/assistant-ui/clarify-tool.test.tsx @@ -298,13 +298,16 @@ describe('ClarifyTool keyboard navigation', () => { }) describe('ClarifyTool pending marker', () => { - it('marks a live choices card so type-to-focus yields its shortcut keys', () => { + it('marks a live choices card with its row count so type-to-focus yields exactly its keys', () => { renderLiveClarify() - // The marker is what `composerFocusBlockedBySurface` keys off of, so the - // global type-to-focus listener stands down and A/B/C… + 1-9 + Enter reach - // the card instead of the composer. - expect(document.querySelector('[data-clarify-choices]')).toBeTruthy() + // `clarifyCardOwnsKey` reads the count off this marker to yield only the + // shortcuts the card renders (A..N + "Other", 1-9, Enter) and let every + // other printable through to the composer. + const card = document.querySelector('[data-clarify-choices]') + + expect(card).toBeTruthy() + expect(Number(card?.getAttribute('data-clarify-choices'))).toBeGreaterThan(0) }) it('does not mark a free-text (no-choice) pending card', () => { diff --git a/apps/desktop/src/components/assistant-ui/clarify-tool.tsx b/apps/desktop/src/components/assistant-ui/clarify-tool.tsx index 6f1442e2cfed..1c143eb0262f 100644 --- a/apps/desktop/src/components/assistant-ui/clarify-tool.tsx +++ b/apps/desktop/src/components/assistant-ui/clarify-tool.tsx @@ -489,6 +489,10 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) { const key = event.key.toLowerCase() + // Only the letters this card actually renders a row for. Anything past + // the last row belongs to the composer — the user is typing a message + // instead of picking an option, and swallowing the keystroke here would + // make the first letter of it vanish. if (key.length === 1 && key >= 'a' && key <= 'z') { const index = key.charCodeAt(0) - 97 @@ -538,11 +542,13 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) { } return ( - // `data-clarify-choices` marks the panel as owning printable/Enter keys - // while its A/B/C… shortcuts are live, so the global type-to-focus listener - // (`composerFocusBlockedBySurface`) stands down and the letters reach this - // card instead of being redirected into the composer. - + // `data-clarify-choices` marks the panel as owning its OWN shortcut keys + // (Enter, and 1..N+1 / A.. for the N choices plus "Other") while they're + // live, so the global type-to-focus listener (`clarifyCardOwnsKey`) yields + // exactly those and lets every other printable through to the composer — + // typing a real message instead of picking an option stays possible. The + // value is the choice count so the check needs no store access. +
{question}