fix(assistant): resolve race condition, loading state, and error feedback in refreshAssistants (janhq#8091)

- Add loading:true at start of refreshAssistants (matching pattern in other hooks)
- Replace console.error with toast.error for user-facing error feedback
- Replace useEffect-derived state with inline computed initialData
- Add ref-based request ID guard to handleEditClick to prevent race conditions
- Add unit tests for refreshAssistants: fetch success, loading:true timing, toast on failure

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

Author: sshirokov

web-app/src/hooks/__tests__/useAssistant.coverage.test.ts

@@ -1,15 +1,24 @@
-import { describe, it, expect, beforeEach, vi } from 'vitest'
+import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'
 import { act, renderHook } from '@testing-library/react'
 import { useAssistant, defaultAssistant } from '../useAssistant'
 
+const mockToast = vi.fn()
+vi.mock('sonner', () => ({
+  toast: {
+    error: mockToast,
+  },
+}))
+
 const mockCreateAssistant = vi.fn().mockResolvedValue(undefined)
 const mockDeleteAssistant = vi.fn().mockResolvedValue(undefined)
+const mockGetAssistants = vi.fn().mockResolvedValue([defaultAssistant])
 
 vi.mock('@/hooks/useServiceHub', () => ({
   getServiceHub: () => ({
     assistants: () => ({
       createAssistant: mockCreateAssistant,
       deleteAssistant: mockDeleteAssistant,
+      getAssistants: mockGetAssistants,
     }),
   }),
 }))
@@ -219,4 +228,51 @@ describe('useAssistant - coverage', () => {
     // currentAssistant should still be jan
     expect(result.current.currentAssistant?.id).toBe('jan')
   })
+
+  it('refreshAssistants should fetch fresh data and update state', async () => {
+    const freshAssistants = [
+      { ...defaultAssistant, name: 'Fresh Jan' },
+      { id: 'a2', name: 'A2', avatar: '', description: '', instructions: '', created_at: 2, parameters: {} },
+    ]
+    mockGetAssistants.mockResolvedValue(freshAssistants as any)
+
+    const { result } = renderHook(() => useAssistant())
+
+    expect(result.current.loading).toBe(true)
+
+    await act(async () => {
+      await result.current.refreshAssistants()
+    })
+
+    expect(result.current.assistants).toEqual(freshAssistants)
+    expect(result.current.loading).toBe(false)
+  })
+
+  it('refreshAssistants should set loading:true before fetch starts', async () => {
+    const deferred = vi.fn().mockReturnValue(new Promise((resolve) => setTimeout(resolve, 100)))
+    mockGetAssistants.mockReturnValue(deferred())
+
+    const { result } = renderHook(() => useAssistant())
+
+    expect(result.current.loading).toBe(true)
+  })
+
+  it('refreshAssistants should show toast error on failure', async () => {
+    mockGetAssistants.mockRejectedValue(new Error('Network error'))
+
+    const { result } = renderHook(() => useAssistant())
+
+    await act(async () => {
+      await result.current.refreshAssistants()
+    })
+
+    expect(mockToast).toHaveBeenCalledWith('Failed to refresh assistants', {
+      description: 'Network error',
+    })
+    expect(result.current.loading).toBe(false)
+  })
+
+  afterEach(() => {
+    mockToast.mockClear()
+  })
 })

web-app/src/hooks/useAssistant.ts

@@ -1,6 +1,7 @@
 import { getServiceHub } from '@/hooks/useServiceHub'
 import { Assistant as CoreAssistant } from '@janhq/core'
 import { create } from 'zustand'
+import { toast } from 'sonner'
 import { localStorageKey } from '@/constants/localStorage'
 
 interface AssistantState {
@@ -217,17 +218,19 @@ export const useAssistant = create<AssistantState>((set, get) => ({
     }
   },
   refreshAssistants: async () => {
+    set({ loading: true })
     try {
       const assistants = await getServiceHub().assistants().getAssistants()
       if (assistants) {
-        // Update the state with fresh assistants
         set({
           assistants,
           loading: false
         })
       }
     } catch (error) {
-      console.error('Failed to refresh assistants:', error)
+      toast.error('Failed to refresh assistants', {
+        description: error instanceof Error ? error.message : String(error),
+      })
       set({ loading: false })
     }
   },

web-app/src/routes/settings/assistant.tsx

@@ -1,6 +1,6 @@
 import { createFileRoute } from '@tanstack/react-router'
 import { route } from '@/constants/routes'
-import { useState, useEffect } from 'react'
+import { useState, useCallback, useRef } from 'react'
 
 import { useAssistant } from '@/hooks/useAssistant'
 
@@ -43,7 +43,7 @@ function AssistantContent() {
   const [editingKey, setEditingKey] = useState<string | null>(null)
   const [deleteConfirmOpen, setDeleteConfirmOpen] = useState(false)
   const [deletingId, setDeletingId] = useState<string | null>(null)
-  const [editingAssistant, setEditingAssistant] = useState<Assistant | null>(null)
+  const refreshRequestRef = useRef(0)
 
   const handleDelete = (id: string) => {
     setDeletingId(id)
@@ -68,27 +68,20 @@ function AssistantContent() {
     setEditingKey(null)
   }
 
-  const handleEditClick = (assistantId: string) => {
+  const handleEditClick = useCallback((assistantId: string) => {
     // Before opening the edit dialog, refresh the assistants to get the latest data
-    refreshAssistants().then(() => {
-      setEditingKey(assistantId)
-      setOpen(true)
-    })
-  }
+    const requestId = ++refreshRequestRef.current
+    refreshAssistants()
+      .then(() => {
+        if (refreshRequestRef.current !== requestId) return
+        setEditingKey(assistantId)
+        setOpen(true)
+      })
+  }, [])
 
   const sortedAssistants = assistants.slice().sort((a, b) => a.created_at - b.created_at)
   const defaultAssistant = sortedAssistants.find((a) => a.id === defaultAssistantId)
 
-  // When editingKey changes, we need to get the latest assistant data
-  useEffect(() => {
-    if (editingKey) {
-      const assistant = assistants.find(a => a.id === editingKey)
-      setEditingAssistant(assistant || null)
-    } else {
-      setEditingAssistant(null)
-    }
-  }, [editingKey, assistants])
-
   return (
     <div className="flex flex-col h-svh w-full">
       <HeaderPage>
@@ -215,7 +208,7 @@ function AssistantContent() {
             open={open}
             onOpenChange={setOpen}
             editingKey={editingKey}
-            initialData={editingAssistant || undefined}
+            initialData={editingKey ? assistants.find(a => a.id === editingKey) : undefined}
             onSave={handleSave}
           />
           <DeleteAssistantDialog