root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #45 from timothyjlaurent/fix/quick-chat-init-race-condition

Browse Source 
fix(dashboard): prevent quick chat init race condition and infinite retry loop
This commit is contained in:
gsxdsm
2026-05-05 17:09:27 -07:00
committed by GitHub
parent 59eeb68198 8853472c2c
commit e018db640e
3 changed files with 262 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
packages/dashboard/app/components/QuickChatFAB.tsx
View File

@@ -954,6 +954,7 @@ export function QuickChatFAB({
startModelChat,
startFreshSession,
refreshSessions,
skipNextSessionInitRef,
} = useQuickChat(projectId, addToast);
const panelRef = useRef<HTMLDivElement | null>(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(() => {

198
packages/dashboard/app/hooks/__tests__/useQuickChat.test.ts
View File

@@ -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 };

52
packages/dashboard/app/hooks/useQuickChat.ts
View File

@@ -58,6 +58,14 @@ export interface UseQuickChatReturn {
refreshSessions: () => Promise<void>;
loadMessages: () => Promise<void>;
reloadMessages: () => Promise<void>;
/**
* 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<boolean>;
}
function normalizeModelSelection(modelProvider?: string, modelId?: string): ModelSelection {
@@ -208,6 +216,24 @@ export function useQuickChat(
const currentSessionKeyRef = useRef<string>("");
const currentSessionTargetRef = useRef<SessionTarget | null>(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<ChatSession | null>(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,