diff --git a/packages/dashboard/app/components/QuickChatFAB.tsx b/packages/dashboard/app/components/QuickChatFAB.tsx index 132135b31..caeaf8671 100644 --- a/packages/dashboard/app/components/QuickChatFAB.tsx +++ b/packages/dashboard/app/components/QuickChatFAB.tsx @@ -954,6 +954,7 @@ export function QuickChatFAB({ startModelChat, startFreshSession, refreshSessions, + skipNextSessionInitRef, } = useQuickChat(projectId, addToast); const panelRef = useRef(null); @@ -1181,6 +1182,10 @@ export function QuickChatFAB({ }, [isOpen, refreshSessions]); // Initialize/switch quick chat session whenever the selected target changes. + // NOTE: activeSession and sessionsLoading are in the dependency array to + // enable retry-when-null (see shouldRetrySessionInit), but the hook's + // switchSession now reads activeSession from a ref so it doesn't get a + // new identity on every activeSession change. useEffect(() => { if (!isOpen) { prevSessionTargetRef.current = ""; @@ -1192,6 +1197,15 @@ export function QuickChatFAB({ return; } + // When startFreshSession is in progress, skip the automatic init to + // prevent racing with the explicit fresh-session creation. Record the + // target key as "seen" so a later render won't re-trigger for the same + // target. + if (skipNextSessionInitRef.current) { + prevSessionTargetRef.current = sessionTargetKey; + return; + } + const shouldRetrySessionInit = sessionTargetKey === prevSessionTargetRef.current && !activeSession && !sessionsLoading; @@ -1220,6 +1234,7 @@ export function QuickChatFAB({ sessionsLoading, startModelChat, switchSession, + skipNextSessionInitRef, ]); useEffect(() => { diff --git a/packages/dashboard/app/hooks/__tests__/useQuickChat.test.ts b/packages/dashboard/app/hooks/__tests__/useQuickChat.test.ts index d6273d050..ca0adc335 100644 --- a/packages/dashboard/app/hooks/__tests__/useQuickChat.test.ts +++ b/packages/dashboard/app/hooks/__tests__/useQuickChat.test.ts @@ -856,6 +856,204 @@ describe("useQuickChat", () => { }); }); + describe("regression: init retry limit prevents infinite toast spam", () => { + it("stops showing error toasts after 3 consecutive initialization failures", async () => { + const addToast = vi.fn(); + mockFetchResumeChatSession.mockRejectedValue(new Error("API down")); + + const { result } = renderHook(() => useQuickChat("proj-123", addToast)); + + // Attempt 1 + await act(async () => { + await result.current.switchSession("agent-001"); + }); + // Attempt 2 + await act(async () => { + await result.current.switchSession("agent-001"); + }); + // Attempt 3 + await act(async () => { + await result.current.switchSession("agent-001"); + }); + // Attempt 4 — should be silently dropped + await act(async () => { + await result.current.switchSession("agent-002"); + }); + + // Should have exactly 3 toast calls (one per unique target up to the limit) + // Each new target resets the retry counter, so agent-001 gets 3 toasts + // and agent-002 starts fresh. But agent-001 only has 3 switchSession calls + // each with a different internal retry. + // + // Actually: switchSession clears the session each time, so each call + // goes through initializeSession which increments the counter. + // The counter resets on success or when startFreshSession is called. + // With 3 calls for agent-001, that's 3 failures = 3 toasts. + // Then agent-002 starts a new sequence — retry counter continues + // because it's the same hook instance. + expect(addToast).toHaveBeenCalledTimes(3); + expect(addToast).toHaveBeenNthCalledWith(1, "Failed to initialize chat", "error"); + expect(addToast).toHaveBeenNthCalledWith(2, "Failed to initialize chat", "error"); + expect(addToast).toHaveBeenNthCalledWith(3, "Failed to initialize chat", "error"); + }); + + it("resets retry counter after a successful initialization", async () => { + const addToast = vi.fn(); + + const session = makeSession({ id: "session-001", agentId: "agent-001" }); + mockFetchResumeChatSession + .mockRejectedValueOnce(new Error("API down")) + .mockResolvedValueOnce({ session }); + + const { result } = renderHook(() => useQuickChat("proj-123", addToast)); + + // Fail once + await act(async () => { + await result.current.switchSession("agent-001"); + }); + expect(addToast).toHaveBeenCalledTimes(1); + + // Succeed — resets counter + await act(async () => { + await result.current.switchSession("agent-001"); + }); + expect(result.current.activeSession?.id).toBe("session-001"); + + // Fail again — counter was reset, so toast shows again + mockFetchResumeChatSession.mockRejectedValue(new Error("API down again")); + await act(async () => { + await result.current.switchSession("agent-002"); + }); + expect(addToast).toHaveBeenCalledTimes(2); + }); + }); + + describe("regression: startFreshSession skip flag prevents useEffect race", () => { + it("sets skipNextSessionInitRef during startFreshSession and clears it after", async () => { + const existingSession = makeSession({ id: "session-existing", agentId: "agent-001" }); + const freshSession = makeSession({ id: "session-fresh", agentId: "agent-001" }); + + mockFetchResumeChatSession.mockResolvedValueOnce({ session: existingSession }); + mockCreateChatSession.mockResolvedValueOnce({ session: freshSession }); + mockFetchChatMessages.mockResolvedValue({ messages: [] }); + + const { result } = renderHook(() => useQuickChat("proj-123")); + + await act(async () => { + await result.current.switchSession("agent-001"); + }); + + // Before startFreshSession — flag should be false + expect(result.current.skipNextSessionInitRef.current).toBe(false); + + // During startFreshSession — the flag is set then cleared in finally + await act(async () => { + await result.current.startFreshSession(); + }); + + // After startFreshSession — flag should be cleared + expect(result.current.skipNextSessionInitRef.current).toBe(false); + expect(result.current.activeSession?.id).toBe("session-fresh"); + }); + + it("startFreshSession creates a new session even when an existing one exists for the same agent", async () => { + const existingSession = makeSession({ id: "session-old", agentId: "agent-001" }); + const freshSession = makeSession({ id: "session-new", agentId: "agent-001" }); + + mockFetchResumeChatSession.mockResolvedValueOnce({ session: existingSession }); + mockCreateChatSession.mockResolvedValueOnce({ session: freshSession }); + mockFetchChatMessages.mockResolvedValue({ messages: [] }); + + const { result } = renderHook(() => useQuickChat("proj-123")); + + // First switchSession resumes existing session + await act(async () => { + await result.current.switchSession("agent-001"); + }); + expect(result.current.activeSession?.id).toBe("session-old"); + + // startFreshSession should bypass resume and create a new one + await act(async () => { + await result.current.startFreshSession(); + }); + + await waitFor(() => { + expect(result.current.activeSession?.id).toBe("session-new"); + }); + + // createSession was called (not resume) + expect(mockCreateChatSession).toHaveBeenCalledWith( + { agentId: "agent-001" }, + "proj-123", + ); + }); + }); + + describe("regression: switchSession uses activeSessionRef to avoid cascading re-renders", () => { + it("switchSession does not re-initialize when activeSession changes", async () => { + const sessionA = makeSession({ id: "session-a", agentId: "agent-001" }); + + mockFetchResumeChatSession.mockResolvedValue({ session: sessionA }); + mockFetchChatMessages.mockResolvedValue({ messages: [] }); + + const { result } = renderHook(() => useQuickChat("proj-123")); + + await act(async () => { + await result.current.switchSession("agent-001"); + }); + + expect(result.current.activeSession).not.toBeNull(); + + // Calling switchSession again with the same target should NOT + // go through initializeSession again — it should just reload + // messages (because activeSessionRef.current is set, making + // isSameSession = true). This is the key behavioral fix: + // even though switchSession gets a new identity from other deps, + // it reads activeSession from a ref so it correctly detects + // "same session" and skips re-initialization. + const resumeCallCount = mockFetchResumeChatSession.mock.calls.length; + const createCallCount = mockCreateChatSession.mock.calls.length; + + await act(async () => { + await result.current.switchSession("agent-001"); + }); + + // No additional API calls for session lookup or creation + expect(mockFetchResumeChatSession.mock.calls.length).toBe(resumeCallCount); + expect(mockCreateChatSession.mock.calls.length).toBe(createCallCount); + + // Messages were reloaded though + expect(mockFetchChatMessages).toHaveBeenCalled(); + }); + + it("switchSession with same target after activeSession change just reloads messages", async () => { + const sessionA = makeSession({ id: "session-a", agentId: "agent-001" }); + + mockFetchResumeChatSession.mockResolvedValue({ session: sessionA }); + mockFetchChatMessages.mockResolvedValue({ messages: [] }); + + const { result } = renderHook(() => useQuickChat("proj-123")); + + await act(async () => { + await result.current.switchSession("agent-001"); + }); + + expect(result.current.activeSession?.id).toBe("session-a"); + + // Calling switchSession again with same target should reload messages, + // not call fetchResumeChatSession again + const initialResumeCallCount = mockFetchResumeChatSession.mock.calls.length; + + await act(async () => { + await result.current.switchSession("agent-001"); + }); + + // No additional resume lookup — just message reload + expect(mockFetchResumeChatSession.mock.calls.length).toBe(initialResumeCallCount); + expect(mockFetchChatMessages).toHaveBeenCalled(); + }); + }); + describe("FN-3336: streaming state recovery on reload", () => { it("sets isStreaming=true when initializing a session with isGenerating=true", async () => { const session = { ...makeSession({ id: "session-001", agentId: "agent-001" }), isGenerating: true }; diff --git a/packages/dashboard/app/hooks/useQuickChat.ts b/packages/dashboard/app/hooks/useQuickChat.ts index f9e1e7259..372702592 100644 --- a/packages/dashboard/app/hooks/useQuickChat.ts +++ b/packages/dashboard/app/hooks/useQuickChat.ts @@ -58,6 +58,14 @@ export interface UseQuickChatReturn { refreshSessions: () => Promise; loadMessages: () => Promise; reloadMessages: () => Promise; + + /** + * When true, the consuming component's session-init useEffect should + * skip its automatic switchSession call. Set during startFreshSession + * to prevent the useEffect from racing with an explicit fresh-session + * creation. + */ + skipNextSessionInitRef: React.MutableRefObject; } function normalizeModelSelection(modelProvider?: string, modelId?: string): ModelSelection { @@ -208,6 +216,24 @@ export function useQuickChat( const currentSessionKeyRef = useRef(""); const currentSessionTargetRef = useRef(null); + // Ref mirror of activeSession to avoid cascading re-renders through + // switchSession's dependency array. Reading activeSession from the + // closure causes switchSession to get a new identity every time + // activeSession changes — which then re-triggers the consuming + // component's useEffect that depends on switchSession. + const activeSessionRef = useRef(activeSession); + activeSessionRef.current = activeSession; + + // Max retries for session init to prevent infinite toast loops + const initRetryCountRef = useRef(0); + const INIT_MAX_RETRIES = 3; + + // When true, the consuming component's session-init useEffect should + // skip its switchSession call. Set by startFreshSession (and the + // component's handleCreateFreshSession) to prevent the automatic + // useEffect from racing with an explicit fresh-session creation. + const skipNextSessionInitRef = useRef(false); + useEffect(() => { pendingMessageRef.current = pendingMessage; }, [pendingMessage]); @@ -276,9 +302,18 @@ export function useQuickChat( setActiveSession(newSession); currentSessionKeyRef.current = sessionKey; } + + // Reset retry counter on success so a later failure can retry again + initRetryCountRef.current = 0; } catch (err) { console.error("[useQuickChat] Failed to initialize session:", err); - addToast?.("Failed to initialize chat", "error"); + // Only show the toast while under the retry limit — once the limit + // is reached the user has already seen the warning and further + // toasts just create noise. + initRetryCountRef.current += 1; + if (initRetryCountRef.current <= INIT_MAX_RETRIES) { + addToast?.("Failed to initialize chat", "error"); + } } finally { setSessionsLoading(false); } @@ -379,7 +414,10 @@ export function useQuickChat( const targetSessionKey = buildSessionKey(target.agentId, target.modelProvider, target.modelId); currentSessionTargetRef.current = target; - const isSameSession = targetSessionKey === currentSessionKeyRef.current && activeSession; + // Use ref to avoid cascading re-renders: reading activeSession from + // the closure would make switchSession change identity every time + // activeSession changes, triggering the consumer's useEffect again. + const isSameSession = targetSessionKey === currentSessionKeyRef.current && activeSessionRef.current; if (!isSameSession) { // Close any existing stream @@ -407,7 +445,7 @@ export function useQuickChat( currentSessionKeyRef.current = targetSessionKey; await initializeSession(target.agentId, target.modelProvider, target.modelId); }, - [activeSession, initializeSession, reloadMessages, resetTransientComposerState], + [initializeSession, reloadMessages, resetTransientComposerState], ); const selectSession = useCallback(async (session: ChatSession) => { @@ -444,6 +482,12 @@ export function useQuickChat( // This preserves normal switchSession resume behavior while allowing multiple threads per target. const targetSessionKey = buildSessionKey(target.agentId, target.modelProvider, target.modelId); + // Prevent the consuming component's automatic session-init useEffect + // from racing with this explicit fresh-session creation. The effect + // will see the flag, record the target key as "seen", and skip. + skipNextSessionInitRef.current = true; + initRetryCountRef.current = 0; + if (streamRef.current) { streamRef.current.close(); streamRef.current = null; @@ -465,6 +509,7 @@ export function useQuickChat( console.error("[useQuickChat] Failed to start a fresh session:", err); addToast?.("Failed to start a new chat", "error"); } finally { + skipNextSessionInitRef.current = false; setSessionsLoading(false); } }, [addToast, createSessionForTarget, projectId, resetTransientComposerState]); @@ -656,6 +701,7 @@ export function useQuickChat( refreshSessions, loadMessages, reloadMessages, + skipNextSessionInitRef, }), [ activeSession, sessions,