mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-29 18:46:59 +00:00
fix(desktop): stop a clarify card swallowing keys it doesn't bind
The card marked itself as owning every printable key, so the first letter of a typed-out answer vanished and the composer never focused. It now publishes its row count and yields only Enter plus the 1..N+1 / A.. rows it actually renders; everything else reaches the composer, which skips the question on send.
This commit is contained in:
parent
251b668f4d
commit
c0dd6e1f3f
4 changed files with 114 additions and 10 deletions
|
|
@ -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<typeof $gateway.get>)
|
||||
}
|
||||
|
||||
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()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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', () => {
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
<ClarifyShell className="grid gap-2 px-2.5 py-2" data-clarify-choices={hasChoices ? '' : undefined}>
|
||||
// `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.
|
||||
<ClarifyShell className="grid gap-2 px-2.5 py-2" data-clarify-choices={hasChoices ? choices.length : undefined}>
|
||||
<div className="flex items-start gap-2">
|
||||
<span className="flex-1 whitespace-pre-wrap font-medium leading-(--conversation-line-height)">{question}</span>
|
||||
<MessageQuestion aria-hidden className="mt-px size-4 shrink-0 text-(--ui-text-tertiary)" />
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue