feat(FN-3609): split TaskDetailModal tests into focused suites
This merge splits the monolithic TaskDetailModal test file into six focused test suites (rendering, attachments/tabs, definition/actions, inline-editing/integrations, models/progress/workflow, responsive/dependencies), adds a changeset for the test isolation baseline, fixes chat SSE optimistic echo Fusion-Task-Id: FN-3609
This commit is contained in:
@@ -152,7 +152,10 @@ Concrete references:
|
|||||||
- Dashboard chat UX lives in `packages/dashboard/app/components/ChatView.tsx` and hooks `useChat.ts` / `useQuickChat.ts`
|
- Dashboard chat UX lives in `packages/dashboard/app/components/ChatView.tsx` and hooks `useChat.ts` / `useQuickChat.ts`
|
||||||
- Main `useChat` session restore/recovery must not reset the active thread during session-list refresh or `chat:session:updated` metadata churn while a response is in flight.
|
- Main `useChat` session restore/recovery must not reset the active thread during session-list refresh or `chat:session:updated` metadata churn while a response is in flight.
|
||||||
- When the active session is still generating after reload/reconnect (`isGenerating: true`), `useChat` keeps recovery streaming state alive ("Connecting…") until the assistant output is observed via SSE or reloaded from messages.
|
- When the active session is still generating after reload/reconnect (`isGenerating: true`), `useChat` keeps recovery streaming state alive ("Connecting…") until the assistant output is observed via SSE or reloaded from messages.
|
||||||
- Chat message submission uses SSE streaming responses from dashboard chat routes
|
- Chat message submission uses SSE streaming responses from dashboard chat routes.
|
||||||
|
- Main-chat optimistic user sends are reconciled against persisted SSE user echoes by content + temp-id replacement, so one user send cannot survive as a duplicate history entry after stream completion.
|
||||||
|
- `streamChatResponse()` must flush trailing buffered SSE data on EOF even without a final newline, so terminal `done`/`error` events are not dropped at chunk boundaries.
|
||||||
|
- Chat generation ownership is isolated by `generationId` (`ChatManager.beginGeneration` + `ChatStreamManager` subscription filters + route preallocation), preventing stale generation terminal events from leaking into a newer active request.
|
||||||
|
|
||||||
### Agent Companies
|
### Agent Companies
|
||||||
|
|
||||||
|
|||||||
@@ -51,6 +51,23 @@ describe("streamChatResponse SSE parser", () => {
|
|||||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("flushes terminal done event when stream ends without final newline", async () => {
|
||||||
|
vi.spyOn(globalThis, "fetch").mockResolvedValue(
|
||||||
|
new Response(createChunkedStream(["event: done\ndata: {\"messageId\":\"msg-tail\"}"]), { status: 200 }),
|
||||||
|
);
|
||||||
|
|
||||||
|
const donePayloads: Array<{ messageId: string }> = [];
|
||||||
|
|
||||||
|
streamChatResponse("s-1", "hi", {
|
||||||
|
onDone: (data) => donePayloads.push(data),
|
||||||
|
onError: vi.fn(),
|
||||||
|
});
|
||||||
|
|
||||||
|
await vi.waitFor(() => {
|
||||||
|
expect(donePayloads).toEqual([{ messageId: "msg-tail" }]);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
it("parses done payload assistant snapshots when present", async () => {
|
it("parses done payload assistant snapshots when present", async () => {
|
||||||
vi.spyOn(globalThis, "fetch").mockResolvedValue(
|
vi.spyOn(globalThis, "fetch").mockResolvedValue(
|
||||||
new Response(
|
new Response(
|
||||||
@@ -70,11 +87,25 @@ describe("streamChatResponse SSE parser", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
await vi.waitFor(() => {
|
await vi.waitFor(() => {
|
||||||
expect(donePayloads).toEqual([{ messageId: "msg-1", message: { id: "msg-1", sessionId: "s-1", role: "assistant", content: "Final reply", thinkingOutput: null, metadata: null, createdAt: "2026-01-01T00:00:00.000Z" } }]);
|
expect(donePayloads).toEqual([
|
||||||
|
{
|
||||||
|
messageId: "msg-1",
|
||||||
|
message: {
|
||||||
|
id: "msg-1",
|
||||||
|
sessionId: "s-1",
|
||||||
|
role: "assistant",
|
||||||
|
content: "Final reply",
|
||||||
|
thinkingOutput: null,
|
||||||
|
metadata: null,
|
||||||
|
createdAt: "2026-01-01T00:00:00.000Z",
|
||||||
|
},
|
||||||
|
},
|
||||||
|
]);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
it("handles done events that have no data payload", async () => { vi.spyOn(globalThis, "fetch").mockResolvedValue(
|
it("handles done events that have no data payload", async () => {
|
||||||
|
vi.spyOn(globalThis, "fetch").mockResolvedValue(
|
||||||
new Response(createChunkedStream(["event: done\n\n"]), { status: 200 }),
|
new Response(createChunkedStream(["event: done\n\n"]), { status: 200 }),
|
||||||
);
|
);
|
||||||
|
|
||||||
|
|||||||
@@ -1354,11 +1354,8 @@ describe("useChat", () => {
|
|||||||
});
|
});
|
||||||
mockFetchChatMessages.mockResolvedValueOnce({ messages: [] });
|
mockFetchChatMessages.mockResolvedValueOnce({ messages: [] });
|
||||||
|
|
||||||
// Track stream handlers separately from SSE handlers
|
|
||||||
let streamDoneHandler: ((data: { messageId: string }) => void) | undefined;
|
|
||||||
mockStreamChatResponse.mockImplementation((_sessionId, _content, handlers) => {
|
mockStreamChatResponse.mockImplementation((_sessionId, _content, handlers) => {
|
||||||
// Capture the onDone handler for stream completion
|
void handlers.onDone;
|
||||||
streamDoneHandler = handlers.onDone;
|
|
||||||
return { close: vi.fn(), isConnected: () => true };
|
return { close: vi.fn(), isConnected: () => true };
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -1376,7 +1373,6 @@ describe("useChat", () => {
|
|||||||
expect(result.current.messages).toHaveLength(0);
|
expect(result.current.messages).toHaveLength(0);
|
||||||
});
|
});
|
||||||
|
|
||||||
// Start streaming
|
|
||||||
await act(async () => {
|
await act(async () => {
|
||||||
await result.current.sendMessage("Hello!");
|
await result.current.sendMessage("Hello!");
|
||||||
});
|
});
|
||||||
@@ -1385,8 +1381,6 @@ describe("useChat", () => {
|
|||||||
expect(result.current.isStreaming).toBe(true);
|
expect(result.current.isStreaming).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
// Simulate SSE event - should not add message during streaming
|
|
||||||
// because isStreaming is true
|
|
||||||
const newMessage = makeMessage({ id: "msg-002", sessionId: "session-001", role: "assistant", content: "Hi" });
|
const newMessage = makeMessage({ id: "msg-002", sessionId: "session-001", role: "assistant", content: "Hi" });
|
||||||
act(() => {
|
act(() => {
|
||||||
subscribeHandler["chat:message:added"]?.({
|
subscribeHandler["chat:message:added"]?.({
|
||||||
@@ -1394,10 +1388,69 @@ describe("useChat", () => {
|
|||||||
} as MessageEvent);
|
} as MessageEvent);
|
||||||
});
|
});
|
||||||
|
|
||||||
// Message should not be added during streaming
|
|
||||||
// (the SSE handler checks isStreaming and skips adding)
|
|
||||||
await waitFor(() => {
|
await waitFor(() => {
|
||||||
expect(result.current.messages).toHaveLength(1); // Only the optimistic user message
|
expect(result.current.messages).toHaveLength(1);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("dedupes optimistic user message when persisted user echo arrives after done", async () => {
|
||||||
|
mockFetchChatSessions.mockResolvedValueOnce({
|
||||||
|
sessions: [makeSession({ id: "session-001", agentId: "agent-001" })],
|
||||||
|
});
|
||||||
|
mockFetchChatMessages.mockResolvedValueOnce({ messages: [] });
|
||||||
|
|
||||||
|
let doneHandler: ((data: { messageId: string }) => void) | undefined;
|
||||||
|
mockStreamChatResponse.mockImplementation((_sessionId, _content, handlers) => {
|
||||||
|
doneHandler = handlers.onDone;
|
||||||
|
return { close: vi.fn(), isConnected: () => true };
|
||||||
|
});
|
||||||
|
|
||||||
|
const { result } = renderHook(() => useChat("proj-123"));
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(result.current.sessions).toHaveLength(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
act(() => {
|
||||||
|
result.current.selectSession("session-001");
|
||||||
|
});
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(result.current.activeSession?.id).toBe("session-001");
|
||||||
|
});
|
||||||
|
|
||||||
|
act(() => {
|
||||||
|
result.current.sendMessage("Hello!");
|
||||||
|
});
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(result.current.messages.filter((message) => message.role === "user")).toHaveLength(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
act(() => {
|
||||||
|
doneHandler?.({ messageId: "msg-assistant-001" });
|
||||||
|
});
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(result.current.isStreaming).toBe(false);
|
||||||
|
expect(result.current.messages).toHaveLength(2);
|
||||||
|
});
|
||||||
|
|
||||||
|
const persistedEcho = makeMessage({
|
||||||
|
id: "msg-user-001",
|
||||||
|
sessionId: "session-001",
|
||||||
|
role: "user",
|
||||||
|
content: "Hello!",
|
||||||
|
});
|
||||||
|
act(() => {
|
||||||
|
subscribeHandler["chat:message:added"]?.({
|
||||||
|
data: JSON.stringify(persistedEcho),
|
||||||
|
} as MessageEvent);
|
||||||
|
});
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
const userMessages = result.current.messages.filter((message) => message.role === "user");
|
||||||
|
expect(userMessages).toHaveLength(1);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -719,8 +719,24 @@ export function useChat(
|
|||||||
// Use ref to get the current value (state may not be updated yet when handler runs)
|
// Use ref to get the current value (state may not be updated yet when handler runs)
|
||||||
if (activeSessionRef.current?.id === message.sessionId && !isStreamingRef.current) {
|
if (activeSessionRef.current?.id === message.sessionId && !isStreamingRef.current) {
|
||||||
setMessages((prev) => {
|
setMessages((prev) => {
|
||||||
// Avoid duplicates
|
// Avoid duplicates by persisted id first.
|
||||||
if (prev.some((m) => m.id === message.id)) return prev;
|
if (prev.some((m) => m.id === message.id)) return prev;
|
||||||
|
|
||||||
|
// Reconcile optimistic local user messages against persisted SSE echoes.
|
||||||
|
// The optimistic message uses a temp id and should be replaced instead of appended.
|
||||||
|
if (message.role === "user") {
|
||||||
|
const optimisticIndex = prev.findIndex((candidate) =>
|
||||||
|
candidate.role === "user"
|
||||||
|
&& candidate.id.startsWith("temp-")
|
||||||
|
&& candidate.content.trim() === message.content.trim(),
|
||||||
|
);
|
||||||
|
if (optimisticIndex >= 0) {
|
||||||
|
const next = [...prev];
|
||||||
|
next[optimisticIndex] = message;
|
||||||
|
return next;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return [...prev, message];
|
return [...prev, message];
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1781,4 +1781,5 @@ describe("ChatManager generation isolation", () => {
|
|||||||
// and not re-deleted/corrupted by sendOne's late finally.
|
// and not re-deleted/corrupted by sendOne's late finally.
|
||||||
expect(chatManager.isGenerating("chat-001")).toBe(false);
|
expect(chatManager.isGenerating("chat-001")).toBe(false);
|
||||||
});
|
});
|
||||||
|
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1193,6 +1193,51 @@ describe("Chat API Routes", () => {
|
|||||||
expect(output).toContain('"messageId":"msg-final"');
|
expect(output).toContain('"messageId":"msg-final"');
|
||||||
expect(output).toContain('"content":"Final reply"');
|
expect(output).toContain('"content":"Final reply"');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("uses the same generation id for subscription and sendMessage", async () => {
|
||||||
|
mockGetSession.mockReturnValue(sampleSession);
|
||||||
|
|
||||||
|
const chatModule = await import("../chat.js");
|
||||||
|
vi.mocked(chatModule.checkRateLimit).mockReturnValue(true);
|
||||||
|
mockBeginGeneration.mockReturnValueOnce({
|
||||||
|
generationId: 42,
|
||||||
|
abortController: new AbortController(),
|
||||||
|
});
|
||||||
|
|
||||||
|
mockSendMessage.mockImplementation(async (sessionId: string) => {
|
||||||
|
mockChatStreamManager.broadcast(sessionId, {
|
||||||
|
type: "done",
|
||||||
|
data: { messageId: "msg-final" },
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
const req = createSSERequest();
|
||||||
|
const { res } = createSSEResponse();
|
||||||
|
|
||||||
|
req.body = { content: "Hello" };
|
||||||
|
req.params = { id: "chat-abc123" };
|
||||||
|
req.query = {} as any;
|
||||||
|
req.headers = {} as any;
|
||||||
|
req.ip = "127.0.0.1";
|
||||||
|
req.socket = { remoteAddress: "127.0.0.1" } as any;
|
||||||
|
|
||||||
|
await invokeSSEHandler(req, res, store, mockChatStore, mockChatManager);
|
||||||
|
|
||||||
|
expect(mockBeginGeneration).toHaveBeenCalledWith("chat-abc123");
|
||||||
|
expect(mockChatStreamManager.subscribe).toHaveBeenCalledWith(
|
||||||
|
"chat-abc123",
|
||||||
|
expect.any(Function),
|
||||||
|
{ generationId: 42 },
|
||||||
|
);
|
||||||
|
expect(mockSendMessage).toHaveBeenCalledWith(
|
||||||
|
"chat-abc123",
|
||||||
|
"Hello",
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
{ generationId: 42 },
|
||||||
|
);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user