mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-30 11:52:04 +00:00
fix(desktop): surface background-session clarify prompts instead of hanging
clarify.request is a one-shot blocking event: the gateway turn blocks on clarify.respond. The desktop handler dropped it for any non-focused session (`if (!isActiveEvent) return`) and stored at most one request in a single global atom, so a background session that asked a clarifying question hung forever and re-focusing it could never recover (the event was already gone). - store/clarify.ts: key pending requests by runtime session id; expose the active session's request via a focus-scoped computed view (ClarifyTool is unchanged). clearClarifyRequest takes an optional session id for targeted clears, with a request-id fallback. - use-message-stream.ts: park every session's clarify (drop the isActiveEvent early return); toast when one lands for a background session since the row otherwise just keeps spinning like normal work. - clarify-tool.tsx: clear by session id so answering one chat can't wipe another's pending request. - store/clarify.test.ts: concurrent independence, focus-scoped view, targeted/stale/fallback clears.
This commit is contained in:
parent
04d620d91f
commit
58eb473baa
4 changed files with 150 additions and 18 deletions
|
|
@ -798,13 +798,16 @@ export function useMessageStream({
|
|||
)
|
||||
}
|
||||
} else if (event.type === 'clarify.request') {
|
||||
if (!isActiveEvent) {
|
||||
return
|
||||
}
|
||||
|
||||
// Surface the clarify tool's overlay. The Python side is blocked on
|
||||
// `clarify.respond`, so without this handler the agent would hang
|
||||
// forever (see tools/clarify_tool.py + tui_gateway/server.py:_block).
|
||||
//
|
||||
// Store the request for whichever session raised it — even a background
|
||||
// one. clarify.request is a one-shot event; if we dropped it for an
|
||||
// unfocused session, that session would block on `clarify.respond`
|
||||
// indefinitely and re-focusing it could never recover (the event is
|
||||
// gone). Parking it per-session lets the user answer once they switch
|
||||
// over; the inline ClarifyTool reads the active session's entry.
|
||||
const requestId = typeof payload?.request_id === 'string' ? payload.request_id : ''
|
||||
const question = typeof payload?.question === 'string' ? payload.question : ''
|
||||
|
||||
|
|
@ -815,6 +818,17 @@ export function useMessageStream({
|
|||
choices: Array.isArray(payload?.choices) ? payload!.choices!.filter(c => typeof c === 'string') : null,
|
||||
sessionId: sessionId ?? null
|
||||
})
|
||||
|
||||
// The transcript only renders the active session, so a background
|
||||
// clarify is otherwise invisible (the row just keeps spinning like
|
||||
// it's working). Nudge the user toward the chat that needs them.
|
||||
if (!isActiveEvent) {
|
||||
notify({
|
||||
kind: 'info',
|
||||
title: 'Another chat needs input',
|
||||
message: 'Hermes asked a question in a background session. Open it to answer.'
|
||||
})
|
||||
}
|
||||
}
|
||||
} else if (event.type === 'error') {
|
||||
const errorMessage = payload?.message || 'Hermes reported an error'
|
||||
|
|
|
|||
|
|
@ -103,7 +103,7 @@ function ClarifyToolPending({ args }: ToolCallMessagePartProps) {
|
|||
answer
|
||||
})
|
||||
triggerHaptic('submit')
|
||||
clearClarifyRequest(matchingRequest.requestId)
|
||||
clearClarifyRequest(matchingRequest.requestId, matchingRequest.sessionId)
|
||||
// The matching tool.complete will land shortly after, swapping this
|
||||
// panel for the ToolFallback view above.
|
||||
} catch (error) {
|
||||
|
|
|
|||
81
apps/desktop/src/store/clarify.test.ts
Normal file
81
apps/desktop/src/store/clarify.test.ts
Normal file
|
|
@ -0,0 +1,81 @@
|
|||
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
|
||||
|
||||
import {
|
||||
$clarifyRequest,
|
||||
$clarifyRequests,
|
||||
type ClarifyRequest,
|
||||
clearClarifyRequest,
|
||||
setClarifyRequest
|
||||
} from './clarify'
|
||||
import { $activeSessionId } from './session'
|
||||
|
||||
function clarify(sessionId: string | null, requestId: string): ClarifyRequest {
|
||||
return {
|
||||
requestId,
|
||||
question: `question-${requestId}`,
|
||||
choices: null,
|
||||
sessionId
|
||||
}
|
||||
}
|
||||
|
||||
describe('clarify store', () => {
|
||||
beforeEach(() => {
|
||||
$clarifyRequests.set({})
|
||||
$activeSessionId.set(null)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
$clarifyRequests.set({})
|
||||
$activeSessionId.set(null)
|
||||
})
|
||||
|
||||
it('keeps clarify requests from concurrent sessions independent', () => {
|
||||
setClarifyRequest(clarify('session-a', 'req-a'))
|
||||
setClarifyRequest(clarify('session-b', 'req-b'))
|
||||
|
||||
expect($clarifyRequests.get()['session-a']?.requestId).toBe('req-a')
|
||||
expect($clarifyRequests.get()['session-b']?.requestId).toBe('req-b')
|
||||
})
|
||||
|
||||
it('exposes only the active session via the focus-scoped view', () => {
|
||||
setClarifyRequest(clarify('session-a', 'req-a'))
|
||||
setClarifyRequest(clarify('session-b', 'req-b'))
|
||||
|
||||
$activeSessionId.set('session-a')
|
||||
expect($clarifyRequest.get()?.requestId).toBe('req-a')
|
||||
|
||||
$activeSessionId.set('session-b')
|
||||
expect($clarifyRequest.get()?.requestId).toBe('req-b')
|
||||
|
||||
$activeSessionId.set('session-c')
|
||||
expect($clarifyRequest.get()).toBeNull()
|
||||
})
|
||||
|
||||
it('clears only the targeted session, leaving the other pending', () => {
|
||||
setClarifyRequest(clarify('session-a', 'req-a'))
|
||||
setClarifyRequest(clarify('session-b', 'req-b'))
|
||||
|
||||
clearClarifyRequest('req-a', 'session-a')
|
||||
|
||||
expect($clarifyRequests.get()['session-a']).toBeUndefined()
|
||||
expect($clarifyRequests.get()['session-b']?.requestId).toBe('req-b')
|
||||
})
|
||||
|
||||
it('ignores a stale clear whose request id no longer matches', () => {
|
||||
setClarifyRequest(clarify('session-a', 'req-a2'))
|
||||
|
||||
clearClarifyRequest('req-a1', 'session-a')
|
||||
|
||||
expect($clarifyRequests.get()['session-a']?.requestId).toBe('req-a2')
|
||||
})
|
||||
|
||||
it('clears by request id across sessions when no session hint is given', () => {
|
||||
setClarifyRequest(clarify('session-a', 'shared'))
|
||||
setClarifyRequest(clarify('session-b', 'other'))
|
||||
|
||||
clearClarifyRequest('shared')
|
||||
|
||||
expect($clarifyRequests.get()['session-a']).toBeUndefined()
|
||||
expect($clarifyRequests.get()['session-b']?.requestId).toBe('other')
|
||||
})
|
||||
})
|
||||
|
|
@ -1,4 +1,6 @@
|
|||
import { atom } from 'nanostores'
|
||||
import { atom, computed } from 'nanostores'
|
||||
|
||||
import { $activeSessionId } from './session'
|
||||
|
||||
export interface ClarifyRequest {
|
||||
requestId: string
|
||||
|
|
@ -7,26 +9,61 @@ export interface ClarifyRequest {
|
|||
sessionId: string | null
|
||||
}
|
||||
|
||||
// Holds the request_id (and metadata) for the most recent in-flight
|
||||
// clarify call. The inline ClarifyTool component (rendered inside the
|
||||
// assistant message stream) reads this to know which request_id to send
|
||||
// back over `clarify.respond`.
|
||||
export const $clarifyRequest = atom<ClarifyRequest | null>(null)
|
||||
// Pending clarify requests keyed by the runtime session id that raised them.
|
||||
// Storing per-session (instead of one shared slot) lets a *background* session
|
||||
// park its clarify request while the user is looking at a different chat, then
|
||||
// resolve it once they switch over — without a second concurrent clarify
|
||||
// clobbering the first. A request with no session id lands under the empty key.
|
||||
const keyFor = (sessionId: string | null | undefined): string => sessionId ?? ''
|
||||
|
||||
export const $clarifyRequests = atom<Record<string, ClarifyRequest>>({})
|
||||
|
||||
// The clarify request for the currently-viewed session. The inline ClarifyTool
|
||||
// only ever mounts inside the active session's transcript, so it reads this
|
||||
// focus-scoped view rather than reaching into the whole map.
|
||||
export const $clarifyRequest = computed(
|
||||
[$clarifyRequests, $activeSessionId],
|
||||
(requests, activeId) => requests[keyFor(activeId)] ?? null
|
||||
)
|
||||
|
||||
export function setClarifyRequest(request: ClarifyRequest): void {
|
||||
$clarifyRequest.set(request)
|
||||
$clarifyRequests.set({ ...$clarifyRequests.get(), [keyFor(request.sessionId)]: request })
|
||||
}
|
||||
|
||||
export function clearClarifyRequest(requestId?: string): void {
|
||||
const current = $clarifyRequest.get()
|
||||
export function clearClarifyRequest(requestId?: string, sessionId?: string | null): void {
|
||||
const requests = $clarifyRequests.get()
|
||||
|
||||
// Targeted clear when the caller knows the session (the common path from the
|
||||
// inline ClarifyTool answering its own request).
|
||||
if (sessionId !== undefined) {
|
||||
const key = keyFor(sessionId)
|
||||
const current = requests[key]
|
||||
|
||||
if (!current || (requestId && current.requestId !== requestId)) {
|
||||
return
|
||||
}
|
||||
|
||||
const next = { ...requests }
|
||||
delete next[key]
|
||||
$clarifyRequests.set(next)
|
||||
|
||||
if (!current) {
|
||||
return
|
||||
}
|
||||
|
||||
if (requestId && current.requestId !== requestId) {
|
||||
return
|
||||
// Fallback with no session hint: drop every entry matching the request id
|
||||
// (or clear all when none is given).
|
||||
const next: Record<string, ClarifyRequest> = {}
|
||||
let changed = false
|
||||
|
||||
for (const [key, value] of Object.entries(requests)) {
|
||||
if (requestId && value.requestId !== requestId) {
|
||||
next[key] = value
|
||||
} else {
|
||||
changed = true
|
||||
}
|
||||
}
|
||||
|
||||
$clarifyRequest.set(null)
|
||||
if (changed) {
|
||||
$clarifyRequests.set(next)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue