Merge commit '60a636efd8e6002a69bae78105fb3ed37de7de46'
This commit is contained in:
@@ -25,7 +25,7 @@ import {
|
|||||||
TriangleAlert,
|
TriangleAlert,
|
||||||
} from "lucide-react";
|
} from "lucide-react";
|
||||||
import { useChat, type ChatMessageInfo, type FailureInfo, type ToolCallInfo } from "../hooks/useChat";
|
import { useChat, type ChatMessageInfo, type FailureInfo, type ToolCallInfo } from "../hooks/useChat";
|
||||||
import { useChatRooms } from "../hooks/useChatRooms";
|
import { RoomMessageDeliveredButReplyFailedError, useChatRooms } from "../hooks/useChatRooms";
|
||||||
import { useChatUnread } from "../hooks/useChatUnread";
|
import { useChatUnread } from "../hooks/useChatUnread";
|
||||||
import { useViewportMode } from "./Header";
|
import { useViewportMode } from "./Header";
|
||||||
import { updateGlobalSettings, type DiscoveredSkill } from "../api";
|
import { updateGlobalSettings, type DiscoveredSkill } from "../api";
|
||||||
@@ -1718,6 +1718,14 @@ export function ChatView({ projectId, addToast, experimentalFeatures }: ChatView
|
|||||||
try {
|
try {
|
||||||
await rooms.sendRoomMessage(trimmed, { files: pendingAttachments.map((attachment) => attachment.file) });
|
await rooms.sendRoomMessage(trimmed, { files: pendingAttachments.map((attachment) => attachment.file) });
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
|
if (error instanceof RoomMessageDeliveredButReplyFailedError) {
|
||||||
|
const message = error.message.trim()
|
||||||
|
? error.message
|
||||||
|
: "Message sent, but assistant reply failed";
|
||||||
|
addToast(`Message sent, but assistant reply failed: ${message}`, "error");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
setMessageInput(previousInput);
|
setMessageInput(previousInput);
|
||||||
const message = error instanceof Error && error.message.trim()
|
const message = error instanceof Error && error.message.trim()
|
||||||
? error.message
|
? error.message
|
||||||
|
|||||||
@@ -5,11 +5,17 @@ import { ChatView } from "../ChatView";
|
|||||||
import * as useChatModule from "../../hooks/useChat";
|
import * as useChatModule from "../../hooks/useChat";
|
||||||
import * as useChatRoomsModule from "../../hooks/useChatRooms";
|
import * as useChatRoomsModule from "../../hooks/useChatRooms";
|
||||||
import type { UseChatReturn, ChatSessionInfo } from "../../hooks/useChat";
|
import type { UseChatReturn, ChatSessionInfo } from "../../hooks/useChat";
|
||||||
import type { UseChatRoomsResult } from "../../hooks/useChatRooms";
|
import { RoomMessageDeliveredButReplyFailedError, type UseChatRoomsResult } from "../../hooks/useChatRooms";
|
||||||
import { _resetInitialViewportHeight } from "../../hooks/useMobileKeyboard";
|
import { _resetInitialViewportHeight } from "../../hooks/useMobileKeyboard";
|
||||||
|
|
||||||
vi.mock("../../hooks/useChat");
|
vi.mock("../../hooks/useChat");
|
||||||
vi.mock("../../hooks/useChatRooms");
|
vi.mock("../../hooks/useChatRooms", async (importOriginal) => {
|
||||||
|
const actual = await importOriginal<typeof import("../../hooks/useChatRooms")>();
|
||||||
|
return {
|
||||||
|
...actual,
|
||||||
|
useChatRooms: vi.fn(),
|
||||||
|
};
|
||||||
|
});
|
||||||
vi.mock("../../hooks/useNavigationHistory", async (importOriginal) => {
|
vi.mock("../../hooks/useNavigationHistory", async (importOriginal) => {
|
||||||
const actual = await importOriginal<typeof import("../../hooks/useNavigationHistory")>();
|
const actual = await importOriginal<typeof import("../../hooks/useNavigationHistory")>();
|
||||||
return {
|
return {
|
||||||
@@ -343,13 +349,11 @@ describe("ChatView — rooms (FN-3805..FN-3811 contract)", () => {
|
|||||||
expect(addToast).not.toHaveBeenCalledWith(expect.stringMatching(/attach/i), "warning");
|
expect(addToast).not.toHaveBeenCalledWith(expect.stringMatching(/attach/i), "warning");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("keeps room composer text and toasts once when room send fails", async () => {
|
it("FN-5360 keeps room composer cleared when delivery succeeded but reply generation failed", async () => {
|
||||||
const addToast = vi.fn();
|
const addToast = vi.fn();
|
||||||
let rejectSend: (error?: unknown) => void;
|
const sendRoomMessage = vi
|
||||||
const sendPromise = new Promise<undefined>((_, reject) => {
|
.fn()
|
||||||
rejectSend = reject;
|
.mockRejectedValueOnce(new RoomMessageDeliveredButReplyFailedError("No active room responders available", "room-a"));
|
||||||
});
|
|
||||||
const sendRoomMessage = vi.fn().mockReturnValue(sendPromise);
|
|
||||||
setup({}, { sendRoomMessage, activeRoom: roomA });
|
setup({}, { sendRoomMessage, activeRoom: roomA });
|
||||||
|
|
||||||
render(<ChatView projectId="proj-123" addToast={addToast} experimentalFeatures={{ chatRooms: true }} />);
|
render(<ChatView projectId="proj-123" addToast={addToast} experimentalFeatures={{ chatRooms: true }} />);
|
||||||
@@ -360,15 +364,46 @@ describe("ChatView — rooms (FN-3805..FN-3811 contract)", () => {
|
|||||||
await waitFor(() => {
|
await waitFor(() => {
|
||||||
expect(sendRoomMessage).toHaveBeenCalledWith("Will retry", { files: [] });
|
expect(sendRoomMessage).toHaveBeenCalledWith("Will retry", { files: [] });
|
||||||
});
|
});
|
||||||
expect(textarea.value).toBe("");
|
await waitFor(() => {
|
||||||
|
expect(textarea.value).toBe("");
|
||||||
|
});
|
||||||
|
expect(addToast).toHaveBeenCalledWith("Message sent, but assistant reply failed: No active room responders available", "error");
|
||||||
|
});
|
||||||
|
|
||||||
rejectSend!(new Error("Room backend failed"));
|
it("FN-5360 restores room composer when delivery fails", async () => {
|
||||||
|
const addToast = vi.fn();
|
||||||
|
const sendRoomMessage = vi.fn().mockRejectedValueOnce(new Error("POST failed"));
|
||||||
|
setup({}, { sendRoomMessage, activeRoom: roomA });
|
||||||
|
|
||||||
|
render(<ChatView projectId="proj-123" addToast={addToast} experimentalFeatures={{ chatRooms: true }} />);
|
||||||
|
|
||||||
|
const textarea = screen.getByTestId("chat-input") as HTMLTextAreaElement;
|
||||||
|
await userEvent.type(textarea, "Will retry{enter}");
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(sendRoomMessage).toHaveBeenCalledWith("Will retry", { files: [] });
|
||||||
|
});
|
||||||
await waitFor(() => {
|
await waitFor(() => {
|
||||||
expect(textarea.value).toBe("Will retry");
|
expect(textarea.value).toBe("Will retry");
|
||||||
});
|
});
|
||||||
expect(addToast).toHaveBeenCalledTimes(1);
|
expect(addToast).toHaveBeenCalledWith("POST failed", "error");
|
||||||
expect(addToast).toHaveBeenCalledWith("Room backend failed", "error");
|
});
|
||||||
|
|
||||||
|
it("clears room composer on Enter when room send succeeds", async () => {
|
||||||
|
const sendRoomMessage = vi.fn().mockResolvedValue(undefined);
|
||||||
|
setup({}, { sendRoomMessage, activeRoom: roomA });
|
||||||
|
|
||||||
|
render(<ChatView projectId="proj-123" addToast={vi.fn()} experimentalFeatures={{ chatRooms: true }} />);
|
||||||
|
|
||||||
|
const textarea = screen.getByTestId("chat-input") as HTMLTextAreaElement;
|
||||||
|
await userEvent.type(textarea, "Delivered{enter}");
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(sendRoomMessage).toHaveBeenCalledWith("Delivered", { files: [] });
|
||||||
|
});
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(textarea.value).toBe("");
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
it("clears room composer optimistically before send resolves", async () => {
|
it("clears room composer optimistically before send resolves", async () => {
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
import { act, renderHook, waitFor } from "@testing-library/react";
|
import { act, renderHook, waitFor } from "@testing-library/react";
|
||||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||||
import type { ChatRoom, ChatRoomMember, ChatRoomMessage } from "@fusion/core";
|
import type { ChatRoom, ChatRoomMember, ChatRoomMessage } from "@fusion/core";
|
||||||
import { useChatRooms } from "../useChatRooms";
|
import { RoomMessageDeliveredButReplyFailedError, useChatRooms } from "../useChatRooms";
|
||||||
import * as apiModule from "../../api";
|
import * as apiModule from "../../api";
|
||||||
import * as sseBusModule from "../../sse-bus";
|
import * as sseBusModule from "../../sse-bus";
|
||||||
import { SWR_CACHE_KEYS } from "../../utils/swrCache";
|
import { SWR_CACHE_KEYS } from "../../utils/swrCache";
|
||||||
@@ -335,14 +335,23 @@ describe("useChatRooms", () => {
|
|||||||
mockUploadChatRoomAttachment.mockRejectedValueOnce(new Error("Upload failed"));
|
mockUploadChatRoomAttachment.mockRejectedValueOnce(new Error("Upload failed"));
|
||||||
mockFetchChatRoomMessages.mockResolvedValueOnce({ messages: [] });
|
mockFetchChatRoomMessages.mockResolvedValueOnce({ messages: [] });
|
||||||
|
|
||||||
|
let uploadError: unknown;
|
||||||
await act(async () => {
|
await act(async () => {
|
||||||
await expect(result.current.sendRoomMessage("hello", { files: [file] })).rejects.toThrow("Failed to upload attachment: bad.txt");
|
try {
|
||||||
|
await result.current.sendRoomMessage("hello", { files: [file] });
|
||||||
|
} catch (error) {
|
||||||
|
uploadError = error;
|
||||||
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
expect(uploadError).toBeInstanceOf(Error);
|
||||||
|
expect((uploadError as Error).message).toBe("Failed to upload attachment: bad.txt");
|
||||||
|
expect(uploadError).not.toBeInstanceOf(RoomMessageDeliveredButReplyFailedError);
|
||||||
|
|
||||||
expect(mockPostChatRoomMessage).not.toHaveBeenCalledWith("room-1", expect.objectContaining({ content: "hello" }), "proj-1");
|
expect(mockPostChatRoomMessage).not.toHaveBeenCalledWith("room-1", expect.objectContaining({ content: "hello" }), "proj-1");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("rolls back optimistic temp message when post fails and transcript refresh fails", async () => {
|
it("rejects with original error when post fails before delivery", async () => {
|
||||||
const active = room("room-1", "one", "2026-05-09T01:00:00.000Z");
|
const active = room("room-1", "one", "2026-05-09T01:00:00.000Z");
|
||||||
mockFetchChatRooms.mockResolvedValueOnce({ rooms: [active] });
|
mockFetchChatRooms.mockResolvedValueOnce({ rooms: [active] });
|
||||||
const { result } = renderHook(() => useChatRooms("proj-1"));
|
const { result } = renderHook(() => useChatRooms("proj-1"));
|
||||||
@@ -356,10 +365,19 @@ describe("useChatRooms", () => {
|
|||||||
mockPostChatRoomMessage.mockRejectedValueOnce(new Error("POST failed"));
|
mockPostChatRoomMessage.mockRejectedValueOnce(new Error("POST failed"));
|
||||||
mockFetchChatRoomMessages.mockRejectedValueOnce(new Error("refresh failed"));
|
mockFetchChatRoomMessages.mockRejectedValueOnce(new Error("refresh failed"));
|
||||||
|
|
||||||
|
let postError: unknown;
|
||||||
await act(async () => {
|
await act(async () => {
|
||||||
await expect(result.current.sendRoomMessage("hello")).rejects.toThrow("POST failed");
|
try {
|
||||||
|
await result.current.sendRoomMessage("hello");
|
||||||
|
} catch (error) {
|
||||||
|
postError = error;
|
||||||
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
expect(postError).toBeInstanceOf(Error);
|
||||||
|
expect((postError as Error).message).toBe("POST failed");
|
||||||
|
expect(postError).not.toBeInstanceOf(RoomMessageDeliveredButReplyFailedError);
|
||||||
|
|
||||||
expect(result.current.messages).toEqual([]);
|
expect(result.current.messages).toEqual([]);
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -384,6 +402,41 @@ describe("useChatRooms", () => {
|
|||||||
expect(mockFetchChatRoomMessages).toHaveBeenLastCalledWith("room-1", { limit: 100, order: "desc" }, "proj-1");
|
expect(mockFetchChatRoomMessages).toHaveBeenLastCalledWith("room-1", { limit: 100, order: "desc" }, "proj-1");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("wraps post-delivery refresh failures with RoomMessageDeliveredButReplyFailedError", async () => {
|
||||||
|
const active = room("room-1", "one", "2026-05-09T01:00:00.000Z");
|
||||||
|
mockFetchChatRooms.mockResolvedValueOnce({ rooms: [active] });
|
||||||
|
const { result } = renderHook(() => useChatRooms("proj-1"));
|
||||||
|
await waitFor(() => expect(result.current.rooms.length).toBe(1));
|
||||||
|
|
||||||
|
mockFetchChatRoomMembers.mockResolvedValueOnce({ members: [] });
|
||||||
|
mockFetchChatRoomMessages.mockResolvedValueOnce({ messages: [] });
|
||||||
|
act(() => result.current.selectRoom("room-1"));
|
||||||
|
await waitFor(() => expect(result.current.activeRoom?.id).toBe("room-1"));
|
||||||
|
|
||||||
|
mockPostChatRoomMessage.mockResolvedValueOnce({ message: roomMessage("msg-user", "room-1", "hello") });
|
||||||
|
mockFetchChatRoomMessages
|
||||||
|
.mockRejectedValueOnce(new Error("refresh failed"))
|
||||||
|
.mockResolvedValueOnce({ messages: [roomMessage("msg-user", "room-1", "hello")] });
|
||||||
|
|
||||||
|
let deliveryError: unknown;
|
||||||
|
await act(async () => {
|
||||||
|
try {
|
||||||
|
await result.current.sendRoomMessage("hello");
|
||||||
|
} catch (error) {
|
||||||
|
deliveryError = error;
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
expect(deliveryError).toBeInstanceOf(RoomMessageDeliveredButReplyFailedError);
|
||||||
|
expect(deliveryError).toEqual(
|
||||||
|
expect.objectContaining({
|
||||||
|
name: "RoomMessageDeliveredButReplyFailedError",
|
||||||
|
roomId: "room-1",
|
||||||
|
message: "refresh failed",
|
||||||
|
}),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
it("refreshes persisted room messages even when room reply generation fails", async () => { const active = room("room-1", "one", "2026-05-09T01:00:00.000Z");
|
it("refreshes persisted room messages even when room reply generation fails", async () => { const active = room("room-1", "one", "2026-05-09T01:00:00.000Z");
|
||||||
mockFetchChatRooms.mockResolvedValueOnce({ rooms: [active] });
|
mockFetchChatRooms.mockResolvedValueOnce({ rooms: [active] });
|
||||||
const { result } = renderHook(() => useChatRooms("proj-1"));
|
const { result } = renderHook(() => useChatRooms("proj-1"));
|
||||||
|
|||||||
@@ -16,6 +16,16 @@ import { readCache, SWR_CACHE_KEYS, SWR_DEFAULT_MAX_AGE_MS, SWR_LONG_MAX_AGE_MS,
|
|||||||
|
|
||||||
const ACTIVE_ROOM_STORAGE_KEY = "fusion:chat-active-room";
|
const ACTIVE_ROOM_STORAGE_KEY = "fusion:chat-active-room";
|
||||||
|
|
||||||
|
export class RoomMessageDeliveredButReplyFailedError extends Error {
|
||||||
|
roomId: string;
|
||||||
|
|
||||||
|
constructor(message: string, roomId: string) {
|
||||||
|
super(message);
|
||||||
|
this.name = "RoomMessageDeliveredButReplyFailedError";
|
||||||
|
this.roomId = roomId;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
export interface UseChatRoomsResult {
|
export interface UseChatRoomsResult {
|
||||||
rooms: ChatRoom[];
|
rooms: ChatRoom[];
|
||||||
roomsLoading: boolean;
|
roomsLoading: boolean;
|
||||||
@@ -205,6 +215,13 @@ export function useChatRooms(
|
|||||||
}
|
}
|
||||||
}, [activeRoomCacheKey, projectId]);
|
}, [activeRoomCacheKey, projectId]);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Sends a room message with optimistic UI.
|
||||||
|
*
|
||||||
|
* Error contract:
|
||||||
|
* - Throws the original error when delivery did not happen (before `postChatRoomMessage` resolves); callers may restore composer text.
|
||||||
|
* - Throws `RoomMessageDeliveredButReplyFailedError` when delivery succeeded but a post-send step failed; callers must keep composer cleared.
|
||||||
|
*/
|
||||||
const sendRoomMessage = useCallback(async (content: string, opts?: { attachments?: ChatAttachment[]; files?: File[] }) => {
|
const sendRoomMessage = useCallback(async (content: string, opts?: { attachments?: ChatAttachment[]; files?: File[] }) => {
|
||||||
const activeRoomSnapshot = activeRoomRef.current;
|
const activeRoomSnapshot = activeRoomRef.current;
|
||||||
const roomId = activeRoomSnapshot?.id;
|
const roomId = activeRoomSnapshot?.id;
|
||||||
@@ -226,6 +243,8 @@ export function useChatRooms(
|
|||||||
setMessages((previous) => [...previous, optimisticMessage]);
|
setMessages((previous) => [...previous, optimisticMessage]);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
let userMessageDelivered = false;
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const uploadedAttachments: ChatAttachment[] = [];
|
const uploadedAttachments: ChatAttachment[] = [];
|
||||||
if (opts?.files?.length) {
|
if (opts?.files?.length) {
|
||||||
@@ -245,6 +264,7 @@ export function useChatRooms(
|
|||||||
content,
|
content,
|
||||||
...(mergedAttachments.length ? { attachments: mergedAttachments } : {}),
|
...(mergedAttachments.length ? { attachments: mergedAttachments } : {}),
|
||||||
}, projectId);
|
}, projectId);
|
||||||
|
userMessageDelivered = true;
|
||||||
|
|
||||||
if (postResult.message?.createdAt && activeRoomSnapshot) {
|
if (postResult.message?.createdAt && activeRoomSnapshot) {
|
||||||
setRooms((previous) => upsertRoom(previous, { ...activeRoomSnapshot, updatedAt: postResult.message.createdAt }));
|
setRooms((previous) => upsertRoom(previous, { ...activeRoomSnapshot, updatedAt: postResult.message.createdAt }));
|
||||||
@@ -271,6 +291,14 @@ export function useChatRooms(
|
|||||||
setMessages((previous) => previous.filter((message) => message.id !== optimisticMessage.id));
|
setMessages((previous) => previous.filter((message) => message.id !== optimisticMessage.id));
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (userMessageDelivered) {
|
||||||
|
const message = error instanceof Error && error.message.trim()
|
||||||
|
? error.message
|
||||||
|
: "Message delivered, but failed to refresh room replies";
|
||||||
|
throw new RoomMessageDeliveredButReplyFailedError(message, roomId);
|
||||||
|
}
|
||||||
|
|
||||||
throw error;
|
throw error;
|
||||||
}
|
}
|
||||||
}, [projectId]);
|
}, [projectId]);
|
||||||
|
|||||||
Reference in New Issue
Block a user