mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-26 17:38:36 +00:00
fix(desktop): keep optional action handlers optional through the latest-actions adapters
The adapters wrapped every field in an arrow function, including the
optional ones. That makes an absent handler unconditionally truthy, and
several children gate on a handler's PRESENCE rather than just calling
it:
- onDismissError -> assistant-message.tsx renders the dismiss button
only when defined
- onRestoreToMessage -> thread/index.tsx gates the restore-confirm flow
- onTranscribeAudio -> use-voice-recorder / use-voice-conversation gate
recording on it
- onLoadMoreMessaging / onLoadMoreProfileSessions -> sidebar paging
So the adapter would paint a dead dismiss button and let voice recording
proceed into a no-op transcription path even when the controller had
deliberately left those handlers off.
Wrap an optional field only when it is currently present, and re-read the
latest value inside the wrapper so the stale-closure fix still applies.
Presence is stable for a given actions object (the controller mutates
fields in place rather than toggling a handler between defined and
undefined), while the closure is what churns — which is exactly what the
indirection re-reads.
Adds two regression tests: absent optional handlers stay undefined, and a
present optional handler still late-binds to the latest closure. The
first was verified to fail against the unconditional-wrapper form.
This commit is contained in:
parent
344976773a
commit
78c06525e8
5 changed files with 73 additions and 5 deletions
|
|
@ -78,4 +78,48 @@ describe('latestActions adapters', () => {
|
|||
expect(staleNavigate).not.toHaveBeenCalled()
|
||||
expect(latestNavigate).toHaveBeenCalledWith(item)
|
||||
})
|
||||
|
||||
// An absent optional handler must stay absent through the adapter. Children
|
||||
// gate on PRESENCE, not just invocation: onDismissError renders the dismiss
|
||||
// button only when defined, onRestoreToMessage gates the restore-confirm
|
||||
// flow, and onTranscribeAudio gates voice recording. Wrapping an undefined
|
||||
// field in an arrow function makes it unconditionally truthy, which would
|
||||
// paint a dead dismiss button and let voice recording run with no
|
||||
// transcription backend.
|
||||
it('leaves absent optional handlers undefined instead of always-truthy wrappers', () => {
|
||||
const chat = makeChatActions()
|
||||
chat.onDismissError = undefined
|
||||
chat.onRestoreToMessage = undefined
|
||||
chat.onTranscribeAudio = undefined
|
||||
|
||||
const adaptedChat = latestChatActions(chat)
|
||||
|
||||
expect(adaptedChat.onDismissError).toBeUndefined()
|
||||
expect(adaptedChat.onRestoreToMessage).toBeUndefined()
|
||||
expect(adaptedChat.onTranscribeAudio).toBeUndefined()
|
||||
|
||||
const sidebar = makeSidebarActions()
|
||||
sidebar.onLoadMoreMessaging = undefined
|
||||
sidebar.onLoadMoreProfileSessions = undefined
|
||||
|
||||
const adaptedSidebar = latestSidebarActions(sidebar)
|
||||
|
||||
expect(adaptedSidebar.onLoadMoreMessaging).toBeUndefined()
|
||||
expect(adaptedSidebar.onLoadMoreProfileSessions).toBeUndefined()
|
||||
})
|
||||
|
||||
it('still late-binds a PRESENT optional handler to the latest closure', async () => {
|
||||
const staleTranscribe = vi.fn(async () => 'stale')
|
||||
const latestTranscribe = vi.fn(async () => 'latest')
|
||||
const actions = makeChatActions()
|
||||
actions.onTranscribeAudio = staleTranscribe
|
||||
|
||||
const adapted = latestChatActions(actions)
|
||||
|
||||
actions.onTranscribeAudio = latestTranscribe
|
||||
|
||||
expect(adapted.onTranscribeAudio).toBeTypeOf('function')
|
||||
await expect(adapted.onTranscribeAudio!(new Blob())).resolves.toBe('latest')
|
||||
expect(staleTranscribe).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -6,7 +6,28 @@ import type { ChatActions, SidebarActions } from './types'
|
|||
* directly, the child keeps the function from the surface's last render and can
|
||||
* submit/click against a stale session closure. These adapters keep a stable
|
||||
* wrapper but dereference the latest field at call time.
|
||||
*
|
||||
* OPTIONAL handlers must stay optional. Several children gate on a handler's
|
||||
* *presence*, not just call it — `onDismissError` renders the dismiss button
|
||||
* only when it exists (assistant-message.tsx), `onRestoreToMessage` gates the
|
||||
* restore-confirm flow (thread/index.tsx), and `onTranscribeAudio` gates voice
|
||||
* recording/conversation (use-voice-recorder, use-voice-conversation). An
|
||||
* unconditional arrow wrapper is always truthy, which would render a dead
|
||||
* dismiss button and let voice recording proceed with no transcription backend.
|
||||
* So wrap an optional field only when it is currently present, and re-read the
|
||||
* latest value inside the wrapper for the stale-closure fix.
|
||||
*/
|
||||
function latestOptional<A extends unknown[], R>(
|
||||
read: () => ((...args: A) => R) | undefined
|
||||
): ((...args: A) => R) | undefined {
|
||||
// Presence is sampled from the object identity the surface currently holds.
|
||||
// The controller mutates fields in place rather than swapping a handler
|
||||
// between defined and undefined, so presence is stable for a given actions
|
||||
// object while the *closure* is what churns — which is exactly what the
|
||||
// indirection below re-reads.
|
||||
return read() ? (...args: A) => read()!(...args) : undefined
|
||||
}
|
||||
|
||||
export function latestChatActions(actions: ChatActions): ChatActions {
|
||||
return {
|
||||
onAddContextRef: (...args) => actions.onAddContextRef(...args),
|
||||
|
|
@ -16,7 +37,7 @@ export function latestChatActions(actions: ChatActions): ChatActions {
|
|||
onBranchInNewChat: (...args) => actions.onBranchInNewChat(...args),
|
||||
onCancel: (...args) => actions.onCancel(...args),
|
||||
onDeleteSelectedSession: (...args) => actions.onDeleteSelectedSession(...args),
|
||||
onDismissError: (...args) => actions.onDismissError?.(...args),
|
||||
onDismissError: latestOptional(() => actions.onDismissError),
|
||||
onEdit: (...args) => actions.onEdit(...args),
|
||||
onPasteClipboardImage: (...args) => actions.onPasteClipboardImage(...args),
|
||||
onPickFiles: (...args) => actions.onPickFiles(...args),
|
||||
|
|
@ -24,13 +45,13 @@ export function latestChatActions(actions: ChatActions): ChatActions {
|
|||
onPickImages: (...args) => actions.onPickImages(...args),
|
||||
onReload: (...args) => actions.onReload(...args),
|
||||
onRemoveAttachment: (...args) => actions.onRemoveAttachment(...args),
|
||||
onRestoreToMessage: (...args) => actions.onRestoreToMessage?.(...args) ?? Promise.resolve(),
|
||||
onRestoreToMessage: latestOptional(() => actions.onRestoreToMessage),
|
||||
onRetryResume: (...args) => actions.onRetryResume(...args),
|
||||
onSteer: (...args) => actions.onSteer(...args),
|
||||
onSubmit: (...args) => actions.onSubmit(...args),
|
||||
onThreadMessagesChange: (...args) => actions.onThreadMessagesChange(...args),
|
||||
onToggleSelectedPin: (...args) => actions.onToggleSelectedPin(...args),
|
||||
onTranscribeAudio: (...args) => actions.onTranscribeAudio?.(...args) ?? Promise.resolve('')
|
||||
onTranscribeAudio: latestOptional(() => actions.onTranscribeAudio)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -39,8 +60,8 @@ export function latestSidebarActions(actions: SidebarActions): SidebarActions {
|
|||
onArchiveSession: (...args) => actions.onArchiveSession(...args),
|
||||
onBranchSession: (...args) => actions.onBranchSession(...args),
|
||||
onDeleteSession: (...args) => actions.onDeleteSession(...args),
|
||||
onLoadMoreMessaging: (...args) => actions.onLoadMoreMessaging?.(...args),
|
||||
onLoadMoreProfileSessions: (...args) => actions.onLoadMoreProfileSessions?.(...args),
|
||||
onLoadMoreMessaging: latestOptional(() => actions.onLoadMoreMessaging),
|
||||
onLoadMoreProfileSessions: latestOptional(() => actions.onLoadMoreProfileSessions),
|
||||
onLoadMoreSessions: (...args) => actions.onLoadMoreSessions(...args),
|
||||
onManageCronJob: (...args) => actions.onManageCronJob(...args),
|
||||
onNavigate: (...args) => actions.onNavigate(...args),
|
||||
|
|
|
|||
|
|
@ -141,6 +141,7 @@ export const ChatRoutesSurface = memo(function ChatRoutesSurface({
|
|||
)
|
||||
|
||||
const chatActions = useMemo(() => latestChatActions(actions), [actions])
|
||||
|
||||
const chatView = (
|
||||
<ChatView
|
||||
gateway={gateway}
|
||||
|
|
|
|||
1
contributors/emails/eagleyouxiang@gmail.com
Normal file
1
contributors/emails/eagleyouxiang@gmail.com
Normal file
|
|
@ -0,0 +1 @@
|
|||
eagle-nyp
|
||||
1
contributors/emails/nypyouxiang@163.com
Normal file
1
contributors/emails/nypyouxiang@163.com
Normal file
|
|
@ -0,0 +1 @@
|
|||
eagle-nyp
|
||||
Loading…
Add table
Add a link
Reference in a new issue