MAINT: Bump eslint-plugin-react-hooks from 5.2.0 to 7.1.1 in /frontend

frontend/package.json

@@ -47,7 +47,7 @@
     "@vitejs/plugin-react": "6.0.2",
     "esbuild": "0.28.0",
     "eslint": "10.4.1",
-    "eslint-plugin-react-hooks": "5.2.0",
+    "eslint-plugin-react-hooks": "7.1.1",
     "eslint-plugin-react-refresh": "0.5.2",
     "globals": "17.6.0",
     "identity-obj-proxy": "3.0.0",

frontend/src/App.tsx

@@ -20,15 +20,18 @@ const AUTO_DISMISS_MS = 5_000
 
 function ConnectionBannerContainer() {
   const { status, reconnectCount } = useConnectionHealth()
-  const [showReconnected, setShowReconnected] = useState(false)
+  // Track how many reconnects the user has already had the banner dismissed for.
+  // `showReconnected` is derived: the banner is visible whenever there are
+  // un-dismissed reconnects. The auto-dismiss timer bumps `dismissedCount` so
+  // we avoid calling setState synchronously in an effect body.
+  const [dismissedCount, setDismissedCount] = useState(0)
+  const showReconnected = reconnectCount > dismissedCount
 
   useEffect(() => {
-    if (reconnectCount > 0) {
-      setShowReconnected(true)
-      const timer = setTimeout(() => setShowReconnected(false), AUTO_DISMISS_MS)
-      return () => clearTimeout(timer)
-    }
-  }, [reconnectCount])
+    if (!showReconnected) return
+    const timer = setTimeout(() => setDismissedCount(reconnectCount), AUTO_DISMISS_MS)
+    return () => clearTimeout(timer)
+  }, [showReconnected, reconnectCount])
 
   if (status === 'connected' && !showReconnected) {
     return null

frontend/src/components/Chat/ChatWindow.tsx

@@ -1,4 +1,4 @@
-import { useState, useRef, useEffect, useCallback } from 'react'
+import { useState, useRef, useEffect, useCallback, useMemo } from 'react'
 import {
   Button,
   Text,
@@ -81,37 +81,43 @@ export default function ChatWindow({
     setAttachmentData(data)
   }, [])
 
-  // Auto-clear stale conversions when their original input no longer matches.
+  // Auto-prune stale conversions whose original input no longer matches.
   // For text: the typed text differs from the captured originalValue.
   // For media: the uploaded base64 changed (or was removed).
-  useEffect(() => {
-    setPieceConversions((prev) => {
-      const entries = Object.entries(prev)
-      if (entries.length === 0) return prev
-      let changed = false
-      const next: Record<string, PieceConversion> = {}
-      for (const [key, conv] of entries) {
-        if (key === 'text') {
-          if (conv.originalValue !== chatInputText) {
-            changed = true
-            continue
-          }
-        } else if (attachmentData[key] !== conv.originalValue) {
-          changed = true
-          continue
-        }
+  // Deriving this rather than syncing via an effect avoids triggering
+  // react-hooks/set-state-in-effect and is the pattern recommended by React
+  // (see frontend-style-guide → "Prefer Derived Values Over Effects").
+  const activePieceConversions = useMemo(() => {
+    const entries = Object.entries(pieceConversions)
+    if (entries.length === 0) return pieceConversions
+    const next: Record<string, PieceConversion> = {}
+    let hasStale = false
+    for (const [key, conv] of entries) {
+      const stillValid = key === 'text'
+        ? conv.originalValue === chatInputText
+        : attachmentData[key] === conv.originalValue
+      if (stillValid) {
         next[key] = conv
+      } else {
+        hasStale = true
       }
-      return changed ? next : prev
-    })
-  }, [chatInputText, attachmentData])
-
-  // Auto-open conversation sidebar when loading a historical attack with multiple conversations
-  useEffect(() => {
-    if (relatedConversationCount && relatedConversationCount > 0) {
-      setIsPanelOpen(true)
     }
-  }, [attackResultId, relatedConversationCount])
+    return hasStale ? next : pieceConversions
+  }, [pieceConversions, chatInputText, attachmentData])
+
+  // Auto-open conversation sidebar when loading a historical attack with multiple
+  // conversations. Uses the "adjust state during render" pattern to avoid
+  // react-hooks/set-state-in-effect.
+  const [autoOpenedForAttack, setAutoOpenedForAttack] = useState<string | null>(null)
+  if (
+    attackResultId
+    && attackResultId !== autoOpenedForAttack
+    && relatedConversationCount
+    && relatedConversationCount > 0
+  ) {
+    setAutoOpenedForAttack(attackResultId)
+    setIsPanelOpen(true)
+  }
   // Set by panel click to bypass the in-flight guard on the next useEffect cycle.
   // This lets users switch to a sending conversation while still protecting
   // optimistic messages when handleSend internally updates activeConversationId.
@@ -126,13 +132,18 @@ export default function ChatWindow({
   // Used to restore the user's input when switching back to an in-flight conversation.
   const pendingUserMessagesRef = useRef<Map<string, Message[]>>(new Map())
 
-  // Clear internal messages when attack state is reset (e.g. New Attack)
-  useEffect(() => {
+  // Clear internal messages when attack state is reset (e.g. New Attack).
+  // Uses the "adjust state during render" pattern (see React docs:
+  // https://react.dev/reference/react/useState#storing-information-from-previous-renders)
+  // instead of a useEffect so we don't trigger react-hooks/set-state-in-effect.
+  const [prevAttackResultId, setPrevAttackResultId] = useState<string | null>(attackResultId)
+  if (attackResultId !== prevAttackResultId) {
+    setPrevAttackResultId(attackResultId)
     if (!attackResultId) {
       setMessages([])
       setLoadedConversationId(null)
     }
-  }, [attackResultId])
+  }
 
   // Load messages for a given conversation
   const loadConversation = useCallback(async (arId: string, convId: string) => {
@@ -180,9 +191,11 @@ export default function ChatWindow({
   // Synchronous loading derivation: if activeConversationId differs from the
   // conversation whose messages we've loaded, we're in a transition gap.
   // This avoids the 1-frame flash between useEffect fire and render.
+  // Reads `sendingConversations` (state) rather than `sendingConvIdsRef` so the
+  // computation stays render-safe (the ref is for handlers/effects only).
   const awaitingConversationLoad = Boolean(
     activeConversationId && activeConversationId !== loadedConversationId
-    && !sendingConvIdsRef.current.has(activeConversationId)
+    && !sendingConversations.has(activeConversationId)
   )
 
   // Handle conversation selection from the panel
@@ -199,7 +212,7 @@ export default function ChatWindow({
     if (!activeTarget) { return }
 
     // Capture all piece conversions upfront before any async work or state clears
-    const conversions = { ...pieceConversions }
+    const conversions = { ...activePieceConversions }
     const textConversion = conversions['text']
     const isTextTextConversion = textConversion?.convertedDataType === 'text'
     const isTextFileConversion = Boolean(textConversion) && !isTextTextConversion
@@ -618,16 +631,16 @@ export default function ChatWindow({
           isConverterPanelOpen={isConverterPanelOpen}
           onInputChange={setChatInputText}
           onAttachmentsChange={handleAttachmentsChange}
-          convertedValue={pieceConversions['text']?.convertedDataType === 'text' ? (pieceConversions['text']?.convertedValue ?? null) : null}
-          originalValue={pieceConversions['text']?.originalValue ?? null}
+          convertedValue={activePieceConversions['text']?.convertedDataType === 'text' ? (activePieceConversions['text']?.convertedValue ?? null) : null}
+          originalValue={activePieceConversions['text']?.originalValue ?? null}
           onClearConversion={() => setPieceConversions((prev) => { const next = { ...prev }; delete next['text']; return next })}
           onConvertedValueChange={(val) => setPieceConversions((prev) => {
             const existing = prev['text']
             if (!existing) return prev
             return { ...prev, text: { ...existing, convertedValue: val } }
           })}
           convertedFileChip={(() => {
-            const tc = pieceConversions['text']
+            const tc = activePieceConversions['text']
             if (!tc || tc.convertedDataType === 'text') return null
             if (!isPathDataType(tc.convertedDataType)) return null
             return {
@@ -637,8 +650,8 @@ export default function ChatWindow({
             }
           })()}
           onClearConvertedFileChip={() => setPieceConversions((prev) => { const next = { ...prev }; delete next['text']; return next })}
-          converterOutputDataTypes={Object.values(pieceConversions).map((c) => c.convertedDataType)}
-          mediaConversions={Object.entries(pieceConversions)
+          converterOutputDataTypes={Object.values(activePieceConversions).map((c) => c.convertedDataType)}
+          mediaConversions={Object.entries(activePieceConversions)
             .filter(([k]) => k !== 'text')
             .map(([k, v]) => ({ pieceType: k, convertedValue: v.convertedValue, convertedDataType: v.convertedDataType }))}
           onClearMediaConversion={(pieceType) => setPieceConversions((prev) => {

frontend/src/components/Chat/ConversationPanel.tsx

@@ -49,34 +49,64 @@ export default function ConversationPanel({
   const styles = useConversationPanelStyles()
   const [conversations, setConversations] = useState<ConversationSummary[]>([])
   const [mainConversationId, setMainConversationId] = useState<string | null>(null)
-  const [isLoading, setIsLoading] = useState(false)
+  // Initialize to `true` only when we will actually fetch on mount so the spinner
+  // shows during the initial fetch without needing a synchronous setState inside
+  // the effect body (which `react-hooks/set-state-in-effect` would flag).
+  const [isLoading, setIsLoading] = useState(() => attackResultId != null)
   const [error, setError] = useState<string | null>(null)
 
-  const fetchConversations = useCallback(async () => {
+  // Counter used by the retry button to re-trigger the fetch effect without
+  // calling setState synchronously in an effect body.
+  const [retryCount, setRetryCount] = useState(0)
+
+  useEffect(() => {
     if (!attackResultId) {
-      setConversations([])
-      setMainConversationId(null)
       return
     }
 
-    setIsLoading(true)
-    setError(null)
-    try {
-      const response = await attacksApi.getConversations(attackResultId)
-      setConversations(response.conversations)
-      setMainConversationId(response.main_conversation_id)
-    } catch (err) {
+    let cancelled = false
+    attacksApi.getConversations(attackResultId)
+      .then((response) => {
+        if (cancelled) return
+        setConversations(response.conversations)
+        setMainConversationId(response.main_conversation_id)
+        setError(null)
+      })
+      .catch((err) => {
+        if (cancelled) return
+        setConversations([])
+        setMainConversationId(null)
+        setError(toApiError(err).detail)
+      })
+      .finally(() => {
+        if (cancelled) return
+        setIsLoading(false)
+      })
+    return () => {
+      cancelled = true
+    }
+  }, [attackResultId, activeConversationId, refreshKey, retryCount])
+
+  // Reset local state when the attack is unloaded so stale data from a prior
+  // attack doesn't briefly appear when the user opens a new one. Uses the
+  // "adjust state during render" pattern instead of a useEffect to satisfy
+  // react-hooks/set-state-in-effect.
+  const [prevAttackResultId, setPrevAttackResultId] = useState<string | null>(attackResultId ?? null)
+  if (attackResultId !== prevAttackResultId) {
+    setPrevAttackResultId(attackResultId ?? null)
+    if (!attackResultId) {
       setConversations([])
       setMainConversationId(null)
-      setError(toApiError(err).detail)
-    } finally {
+      setError(null)
       setIsLoading(false)
     }
-  }, [attackResultId])
+  }
 
-  useEffect(() => {
-    fetchConversations()
-  }, [fetchConversations, activeConversationId, refreshKey])
+  const handleRetry = useCallback(() => {
+    setIsLoading(true)
+    setError(null)
+    setRetryCount((c) => c + 1)
+  }, [])
 
   // Expose refresh via a data attribute on the root element so parent can call it
   // Actually, we'll handle refresh via the attackConversationId dependency
@@ -145,7 +175,7 @@ export default function ConversationPanel({
               appearance="primary"
               size="small"
               icon={<ArrowSyncRegular />}
-              onClick={fetchConversations}
+              onClick={handleRetry}
               data-testid="conversation-retry-btn"
             >
               Retry