mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-13 09:01:54 +00:00
Merge pull request #45255 from NousResearch/bb/desktop-stuck-tool-rows
fix(desktop): dismiss settled tool rows (persistent, caret-safe)
This commit is contained in:
commit
0a7a81835b
4 changed files with 234 additions and 4 deletions
|
|
@ -1,9 +1,10 @@
|
|||
import { AssistantRuntimeProvider, type ThreadMessage, useExternalStoreRuntime } from '@assistant-ui/react'
|
||||
import { cleanup, render, waitFor } from '@testing-library/react'
|
||||
import { cleanup, fireEvent, render, screen, waitFor } from '@testing-library/react'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { clearAllPrompts, setApprovalRequest } from '@/store/prompts'
|
||||
import { $activeSessionId } from '@/store/session'
|
||||
import { clearDismissedToolRows } from '@/store/tool-dismiss'
|
||||
import { $toolDisclosureStates } from '@/store/tool-view'
|
||||
|
||||
import { Thread } from './thread'
|
||||
|
|
@ -104,6 +105,84 @@ function groupedPendingMessage(): ThreadMessage {
|
|||
} as ThreadMessage
|
||||
}
|
||||
|
||||
function pendingOnlyMessage(): ThreadMessage {
|
||||
return {
|
||||
id: 'assistant-pending-only',
|
||||
role: 'assistant',
|
||||
content: [
|
||||
{
|
||||
type: 'tool-call',
|
||||
toolCallId: 'term-only',
|
||||
toolName: 'terminal',
|
||||
args: { command: 'sleep 10' },
|
||||
argsText: JSON.stringify({ command: 'sleep 10' })
|
||||
}
|
||||
],
|
||||
status: { type: 'running' },
|
||||
createdAt,
|
||||
metadata: {
|
||||
unstable_state: null,
|
||||
unstable_annotations: [],
|
||||
unstable_data: [],
|
||||
steps: [],
|
||||
custom: {}
|
||||
}
|
||||
} as ThreadMessage
|
||||
}
|
||||
|
||||
function completedOnlyMessage(): ThreadMessage {
|
||||
return {
|
||||
id: 'assistant-completed-only',
|
||||
role: 'assistant',
|
||||
content: [
|
||||
{
|
||||
type: 'tool-call',
|
||||
toolCallId: 'read-only',
|
||||
toolName: 'read_file',
|
||||
args: { path: '/etc/hosts' },
|
||||
argsText: JSON.stringify({ path: '/etc/hosts' }),
|
||||
result: { content: '127.0.0.1 localhost' }
|
||||
}
|
||||
],
|
||||
status: { type: 'complete', reason: 'stop' },
|
||||
createdAt,
|
||||
metadata: {
|
||||
unstable_state: null,
|
||||
unstable_annotations: [],
|
||||
unstable_data: [],
|
||||
steps: [],
|
||||
custom: {}
|
||||
}
|
||||
} as ThreadMessage
|
||||
}
|
||||
|
||||
function failedOnlyMessage(): ThreadMessage {
|
||||
return {
|
||||
id: 'assistant-failed-only',
|
||||
role: 'assistant',
|
||||
content: [
|
||||
{
|
||||
type: 'tool-call',
|
||||
toolCallId: 'term-failed',
|
||||
toolName: 'terminal',
|
||||
args: { command: 'exit 1' },
|
||||
argsText: JSON.stringify({ command: 'exit 1' }),
|
||||
isError: true,
|
||||
result: { stderr: 'boom' }
|
||||
}
|
||||
],
|
||||
status: { type: 'complete', reason: 'stop' },
|
||||
createdAt,
|
||||
metadata: {
|
||||
unstable_state: null,
|
||||
unstable_annotations: [],
|
||||
unstable_data: [],
|
||||
steps: [],
|
||||
custom: {}
|
||||
}
|
||||
} as ThreadMessage
|
||||
}
|
||||
|
||||
function GroupHarness({ message }: { message: ThreadMessage }) {
|
||||
const runtime = useExternalStoreRuntime<ThreadMessage>({
|
||||
messages: [message],
|
||||
|
|
@ -122,12 +201,14 @@ beforeEach(() => {
|
|||
clearAllPrompts()
|
||||
$activeSessionId.set('sess-1')
|
||||
$toolDisclosureStates.set({})
|
||||
clearDismissedToolRows()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
cleanup()
|
||||
clearAllPrompts()
|
||||
$activeSessionId.set(null)
|
||||
clearDismissedToolRows()
|
||||
})
|
||||
|
||||
describe('flat tool list approval surfacing', () => {
|
||||
|
|
@ -155,4 +236,64 @@ describe('flat tool list approval surfacing', () => {
|
|||
expect(bar?.closest('[hidden]')).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
it('lets completed tool rows be dismissed', async () => {
|
||||
const { container } = render(<GroupHarness message={completedOnlyMessage()} />)
|
||||
|
||||
const dismiss = await screen.findByLabelText('Dismiss')
|
||||
|
||||
expect(container.querySelectorAll('[data-slot="tool-block"]').length).toBeGreaterThan(1)
|
||||
|
||||
fireEvent.click(dismiss)
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByLabelText('Dismiss')).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
it('keeps a dismissed row hidden after a remount (virtualization)', async () => {
|
||||
// The thread virtualizes, so a row's component unmounts/remounts as it
|
||||
// scrolls. Dismissal must persist across that — component-local state would
|
||||
// forget it and the row would pop back. Simulate the remount by unmounting
|
||||
// and rendering the same message fresh.
|
||||
const first = render(<GroupHarness message={completedOnlyMessage()} />)
|
||||
|
||||
fireEvent.click(await screen.findByLabelText('Dismiss'))
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByLabelText('Dismiss')).toBeNull()
|
||||
})
|
||||
|
||||
first.unmount()
|
||||
|
||||
const { container } = render(<GroupHarness message={completedOnlyMessage()} />)
|
||||
|
||||
await waitFor(() => {
|
||||
expect(container.querySelectorAll('[data-slot="tool-block"]').length).toBeGreaterThan(0)
|
||||
})
|
||||
|
||||
expect(screen.queryByLabelText('Dismiss')).toBeNull()
|
||||
})
|
||||
|
||||
it('lets failed tool rows be dismissed', async () => {
|
||||
render(<GroupHarness message={failedOnlyMessage()} />)
|
||||
|
||||
const dismiss = await screen.findByLabelText('Dismiss')
|
||||
|
||||
fireEvent.click(dismiss)
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.queryByLabelText('Dismiss')).toBeNull()
|
||||
})
|
||||
})
|
||||
|
||||
it('does not show dismiss for pending tool rows', async () => {
|
||||
const { container } = render(<GroupHarness message={pendingOnlyMessage()} />)
|
||||
|
||||
await waitFor(() => {
|
||||
expect(container.querySelectorAll('[data-slot="tool-block"]').length).toBeGreaterThan(0)
|
||||
})
|
||||
|
||||
expect(screen.queryByLabelText('Dismiss')).toBeNull()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -12,16 +12,20 @@ import { DiffLines } from '@/components/chat/diff-lines'
|
|||
import { DisclosureRow } from '@/components/chat/disclosure-row'
|
||||
import { PreviewAttachment } from '@/components/chat/preview-attachment'
|
||||
import { ZoomableImage } from '@/components/chat/zoomable-image'
|
||||
import { Button } from '@/components/ui/button'
|
||||
import { Codicon } from '@/components/ui/codicon'
|
||||
import { CopyButton } from '@/components/ui/copy-button'
|
||||
import { FadeText } from '@/components/ui/fade-text'
|
||||
import { GlyphSpinner } from '@/components/ui/glyph-spinner'
|
||||
import { ToolIcon } from '@/components/ui/tool-icon'
|
||||
import { Tip } from '@/components/ui/tooltip'
|
||||
import { useI18n } from '@/i18n'
|
||||
import { PrettyLink, LinkifiedText as SharedLinkifiedText, urlSlugTitleLabel } from '@/lib/external-link'
|
||||
import { AlertCircle, CheckCircle2 } from '@/lib/icons'
|
||||
import { useEnterAnimation } from '@/lib/use-enter-animation'
|
||||
import { cn } from '@/lib/utils'
|
||||
import { $toolInlineDiffs } from '@/store/tool-diffs'
|
||||
import { $toolRowDismissed, dismissToolRow } from '@/store/tool-dismiss'
|
||||
import { $toolDisclosureOpen, $toolViewMode, setToolDisclosureOpen } from '@/store/tool-view'
|
||||
|
||||
import { PendingToolApproval } from './tool-approval'
|
||||
|
|
@ -193,13 +197,16 @@ function useDisclosureOpen(disclosureId: string, fallbackOpen = false): boolean
|
|||
function ToolEntry({ part }: ToolEntryProps) {
|
||||
const { t } = useI18n()
|
||||
const copy = t.assistant.tool
|
||||
const statusCopy = t.statusStack
|
||||
const messageId = useAuiState(s => s.message.id)
|
||||
const messageRunning = useAuiState(selectMessageRunning)
|
||||
const embedded = useContext(ToolEmbedContext)
|
||||
const toolViewMode = useStore($toolViewMode)
|
||||
const disclosureId = `tool-entry:${messageId}:${toolPartDisclosureId(part)}`
|
||||
const dismissed = useStore($toolRowDismissed(disclosureId))
|
||||
const open = useDisclosureOpen(disclosureId)
|
||||
const isPending = messageRunning && part.result === undefined
|
||||
const canDismiss = !isPending && !embedded
|
||||
// Only animate entries that mount while their message is actively
|
||||
// streaming — historical sessions mount with `messageRunning === false`,
|
||||
// so they paint statically without a settle cascade. The wrapping group
|
||||
|
|
@ -282,9 +289,33 @@ function ToolEntry({ part }: ToolEntryProps) {
|
|||
// the disclosure caret hard to hit. Copy now lives in the expanded body's
|
||||
// top-right, where it can't fight the caret for the right edge.
|
||||
const trailing =
|
||||
isPending && !embedded ? (
|
||||
<ActivityTimerText className={TOOL_HEADER_DURATION_CLASS} seconds={elapsed} />
|
||||
) : undefined
|
||||
isPending && !embedded ? <ActivityTimerText className={TOOL_HEADER_DURATION_CLASS} seconds={elapsed} /> : undefined
|
||||
|
||||
// Once a turn has settled, a hover/focus-revealed dismiss lets the user clear
|
||||
// a completed/failed row that would otherwise sit at the tail of the chat.
|
||||
// It goes in the in-flow `action` slot (not `trailing`) so it can't overlap
|
||||
// the disclosure caret's hit-target — see the comment above `trailing`.
|
||||
const dismissAction = canDismiss ? (
|
||||
<Tip label={statusCopy.dismiss}>
|
||||
<Button
|
||||
aria-label={statusCopy.dismiss}
|
||||
className="size-5 rounded-md text-(--ui-text-tertiary) opacity-0 transition-opacity hover:text-(--ui-text-primary) hover:opacity-100 group-hover/disclosure-row:opacity-80 group-focus-within/disclosure-row:opacity-80"
|
||||
onClick={event => {
|
||||
event.stopPropagation()
|
||||
dismissToolRow(disclosureId)
|
||||
}}
|
||||
size="icon-xs"
|
||||
type="button"
|
||||
variant="ghost"
|
||||
>
|
||||
<Codicon name="close" size="0.75rem" />
|
||||
</Button>
|
||||
</Tip>
|
||||
) : undefined
|
||||
|
||||
if (dismissed) {
|
||||
return null
|
||||
}
|
||||
|
||||
return (
|
||||
<div
|
||||
|
|
@ -297,6 +328,7 @@ function ToolEntry({ part }: ToolEntryProps) {
|
|||
>
|
||||
<div className={cn(open && 'border-b border-(--ui-stroke-tertiary) px-2 py-1.5')}>
|
||||
<DisclosureRow
|
||||
action={dismissAction}
|
||||
onToggle={hasExpandableContent ? () => setToolDisclosureOpen(disclosureId, !open) : undefined}
|
||||
open={open}
|
||||
trailing={trailing}
|
||||
|
|
|
|||
|
|
@ -14,12 +14,19 @@ import { cn } from '@/lib/utils'
|
|||
// title text, NOT the full row — and reaches just past the chevron with
|
||||
// `-mx-1.5 px-1.5` so it reads as a soft hit-target rather than a slab
|
||||
// stretching to the message edge.
|
||||
// - `trailing` overlays the right edge (absolute) and must stay
|
||||
// non-interactive (e.g. a duration timer) — an opacity-0-but-clickable
|
||||
// control there steals clicks from the caret. Interactive controls go in
|
||||
// `action`, which lays out *in flow* at the far right so it never sits on
|
||||
// top of the caret's hit-target, no matter how long the title is.
|
||||
export function DisclosureRow({
|
||||
action,
|
||||
children,
|
||||
onToggle,
|
||||
open,
|
||||
trailing
|
||||
}: {
|
||||
action?: ReactNode
|
||||
children: ReactNode
|
||||
onToggle?: () => void
|
||||
open: boolean
|
||||
|
|
@ -55,6 +62,11 @@ export function DisclosureRow({
|
|||
</span>
|
||||
)}
|
||||
</button>
|
||||
{action && (
|
||||
<span className="ml-auto flex h-(--conversation-line-height) shrink-0 items-center self-start pl-1.5">
|
||||
{action}
|
||||
</span>
|
||||
)}
|
||||
{trailing && (
|
||||
<span className="absolute right-1 top-0 flex h-(--conversation-line-height) items-center">{trailing}</span>
|
||||
)}
|
||||
|
|
|
|||
45
apps/desktop/src/store/tool-dismiss.ts
Normal file
45
apps/desktop/src/store/tool-dismiss.ts
Normal file
|
|
@ -0,0 +1,45 @@
|
|||
import { atom, computed, type ReadableAtom } from 'nanostores'
|
||||
|
||||
type DismissedToolRows = Record<string, true>
|
||||
|
||||
// Tool rows the user has locally hidden via a row's dismiss control. This is a
|
||||
// *view-only* hide: the underlying tool call still lives in the stored chat
|
||||
// history, but once a turn has settled the user can clear a completed/failed
|
||||
// row out of the way so it stops sitting at the tail of the conversation.
|
||||
//
|
||||
// Kept in module memory (not localStorage, unlike $toolDisclosureStates) on
|
||||
// purpose: the thread is virtualized, so a dismissed row's component unmounts
|
||||
// and remounts as it scrolls — component-local state would forget the dismissal
|
||||
// and the row would pop back. Storing it here survives those remounts for the
|
||||
// life of the app session, while a reload restores every row in place rather
|
||||
// than permanently rewriting history from a stray click.
|
||||
export const $dismissedToolRows = atom<DismissedToolRows>({})
|
||||
|
||||
const dismissedCache = new Map<string, ReadableAtom<boolean>>()
|
||||
|
||||
export function $toolRowDismissed(id: string): ReadableAtom<boolean> {
|
||||
let cached = dismissedCache.get(id)
|
||||
|
||||
if (!cached) {
|
||||
cached = computed($dismissedToolRows, rows => Boolean(rows[id]))
|
||||
dismissedCache.set(id, cached)
|
||||
}
|
||||
|
||||
return cached
|
||||
}
|
||||
|
||||
export function dismissToolRow(id: string) {
|
||||
if (!id || $dismissedToolRows.get()[id]) {
|
||||
return
|
||||
}
|
||||
|
||||
$dismissedToolRows.set({ ...$dismissedToolRows.get(), [id]: true })
|
||||
}
|
||||
|
||||
export function clearDismissedToolRows() {
|
||||
if (Object.keys($dismissedToolRows.get()).length === 0) {
|
||||
return
|
||||
}
|
||||
|
||||
$dismissedToolRows.set({})
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue