From 78c06525e8e955e06a007b07b347c679f3977c3e Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sat, 25 Jul 2026 12:50:12 -0700 Subject: [PATCH] fix(desktop): keep optional action handlers optional through the latest-actions adapters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../src/app/contrib/latest-actions.test.ts | 44 +++++++++++++++++++ .../desktop/src/app/contrib/latest-actions.ts | 31 ++++++++++--- apps/desktop/src/app/contrib/surfaces.tsx | 1 + contributors/emails/eagleyouxiang@gmail.com | 1 + contributors/emails/nypyouxiang@163.com | 1 + 5 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 contributors/emails/eagleyouxiang@gmail.com create mode 100644 contributors/emails/nypyouxiang@163.com diff --git a/apps/desktop/src/app/contrib/latest-actions.test.ts b/apps/desktop/src/app/contrib/latest-actions.test.ts index b0fecc28efed..9ff335054f37 100644 --- a/apps/desktop/src/app/contrib/latest-actions.test.ts +++ b/apps/desktop/src/app/contrib/latest-actions.test.ts @@ -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() + }) }) diff --git a/apps/desktop/src/app/contrib/latest-actions.ts b/apps/desktop/src/app/contrib/latest-actions.ts index 91750243a127..4407739398a9 100644 --- a/apps/desktop/src/app/contrib/latest-actions.ts +++ b/apps/desktop/src/app/contrib/latest-actions.ts @@ -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( + 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), diff --git a/apps/desktop/src/app/contrib/surfaces.tsx b/apps/desktop/src/app/contrib/surfaces.tsx index 0d95bdf6e78f..852c5311a29c 100644 --- a/apps/desktop/src/app/contrib/surfaces.tsx +++ b/apps/desktop/src/app/contrib/surfaces.tsx @@ -141,6 +141,7 @@ export const ChatRoutesSurface = memo(function ChatRoutesSurface({ ) const chatActions = useMemo(() => latestChatActions(actions), [actions]) + const chatView = (