Skip to content

fix(assistant): handle nullish state and optional callbacks in assistantSessionState - #1792

Closed
hsusul wants to merge 1 commit into
OpenWhispr:mainfrom
hsusul:fix/1791-assistant-session-state-null-safety
Closed

fix(assistant): handle nullish state and optional callbacks in assistantSessionState#1792
hsusul wants to merge 1 commit into
OpenWhispr:mainfrom
hsusul:fix/1791-assistant-session-state-null-safety

Conversation

@hsusul

@hsusul hsusul commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Fixes #1791

Problem

In src/helpers/assistantSessionState.js:

  1. closeAssistantSessionState(state) accessed state.conversationId directly, throwing a TypeError when called with null or undefined.
  2. resolveAssistantPanelBusy(params) destructured properties directly from params, throwing a TypeError when params was null or undefined.
  3. restoreAssistantConversation called callback functions (onReady(), onError(error), onReset()) directly, throwing a TypeError when any callback was omitted or undefined.

Solution

  • Added optional chaining in closeAssistantSessionState (state?.conversationId ?? null).
  • Defaulted params to {} in resolveAssistantPanelBusy.
  • Used optional invocation for callbacks (onReady?.(), onError?.(error), onReset?.(), loadConversation?.()) in restoreAssistantConversation.
  • Added unit tests in test/helpers/assistantSessionState.test.js.

Verification

  • node --test test/helpers/assistantSessionState.test.js (passes, 7/7 tests)
  • npm run typecheck (passes, 0 errors)
  • npm run lint (passes, 0 errors)
  • npm run i18n:check (passes)
  • npm run build:renderer (passes)
  • git diff --check (clean)

@Chadpiha

Copy link
Copy Markdown
Collaborator

Thanks @hsusul! Closing with #1791 (general reasoning in the #1840 note).

All three helpers are called from exactly two files, always with inline object literals — and restoreAssistantConversation's caller passes every callback, so none is optional in practice. The optional-invocation change is the part we'd most want to avoid: loadConversation?.() would let a future wiring bug report "restored" without actually loading a conversation, and onError?.() would swallow the error signal — silent wrong answers instead of a loud crash at the bug site.

Thanks for the contribution! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

assistantSessionState helpers throw TypeError on nullish parameters or missing callbacks

2 participants