diff --git a/packages/views/chat/components/chat-input-draft-isolation.test.tsx b/packages/views/chat/components/chat-input-draft-isolation.test.tsx index aa0d223ac0..93f47eabc4 100644 --- a/packages/views/chat/components/chat-input-draft-isolation.test.tsx +++ b/packages/views/chat/components/chat-input-draft-isolation.test.tsx @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { act, render } from "@testing-library/react"; +import { act, fireEvent, render, screen } from "@testing-library/react"; import { I18nProvider } from "@multica/core/i18n/react"; +import type { UploadResult } from "@multica/core/hooks/use-file-upload"; import enCommon from "../../locales/en/common.json"; import enChat from "../../locales/en/chat.json"; @@ -39,11 +40,32 @@ const mockSetContent = vi.hoisted(() => vi.fn()); vi.mock("@tanstack/react-query", () => ({ useQueryClient: () => ({}), })); +// Captures what ContentEditor wires into its extensions. `onSubmitRef` is the +// Mod+Enter path — it bypasses the SubmitButton entirely, which is why the +// send guards live inside the handler and not only in the button's disabled +// state. +const capturedExtOptions = vi.hoisted<{ + current?: { onSubmitRef?: { current?: () => void } }; +}>(() => ({})); vi.mock("../../editor/extensions", () => ({ - createEditorExtensions: () => [], + createEditorExtensions: (options: { onSubmitRef?: { current?: () => void } }) => { + capturedExtOptions.current = options; + return []; + }, })); +// Mirrors the real contract that matters here: `handler` is captured at CALL +// time (content-editor passes `onUploadFileRef.current` when the upload +// starts), and on completion the real implementation dispatches into the SAME +// editor instance — which is what fires onUpdate. Tests drive that dispatch +// explicitly via `finishUpload()`. vi.mock("../../editor/extensions/file-upload", () => ({ - uploadAndInsertFile: vi.fn(), + uploadAndInsertFile: async ( + _editor: unknown, + file: File, + handler: (f: File) => Promise, + ) => { + await handler(file); + }, })); vi.mock("../../editor/utils/preprocess", () => ({ preprocessMarkdown: (value: string) => value, @@ -144,6 +166,29 @@ import { useChatStore } from "@multica/core/chat"; const TEST_RESOURCES = { en: { common: enCommon, chat: enChat } }; +function makeUpload(id: string, filename: string): UploadResult { + const link = `/api/attachments/${id}/download`; + return { + id, + filename, + workspace_id: "ws-1", + issue_id: null, + comment_id: null, + chat_session_id: null, + chat_message_id: null, + uploader_type: "member", + uploader_id: "user-1", + url: link, + download_url: link, + markdown_url: link, + content_type: "image/png", + size_bytes: 1, + created_at: new Date(0).toISOString(), + markdownLink: link, + link, + }; +} + function store() { return useChatStore.getState() as unknown as { activeSessionId: string | null; @@ -153,14 +198,43 @@ function store() { }; } -function element() { +function element(props: Partial> = {}) { return ( - + ); } +/** ContentEditor publishes upload-queue flips off `editor.on("transaction")`, + * not off onUpdate — every insert/dispatch below is a transaction in the real + * editor, so the tests emit one too. */ +function emitTransaction() { + act(() => { + for (const listener of [...transactionListeners.current]) listener(); + }); +} + +/** Put the editor in the state a live upload leaves it in: file-upload.ts + * inserts a blob placeholder node (attrs.uploading = true) via a transaction, + * so `hasUploadingNode` — and therefore Guard 0, `hasActiveUploads`, and the + * host's upload gate — all report an upload in flight. */ +function beginUpload(markdown: string) { + editorState.uploadingNodes = [{ attrs: { uploading: true } }]; + type(markdown); + emitTransaction(); +} + +/** What file-upload.ts does on completion: swap the blob for the CDN URL and + * clear the uploading flag, then dispatch into the SAME editor instance. That + * dispatch both fires onUpdate and, as a transaction, is how ContentEditor + * publishes the flip that un-gates the host. */ +function finishUpload(markdown: string) { + editorState.uploadingNodes = []; + type(markdown); + emitTransaction(); +} + /** Simulate real typing: move the document, then fire the editor's own * (debounced) onUpdate exactly as Tiptap would. */ function type(markdown: string) { @@ -249,6 +323,129 @@ describe("ChatInput draft isolation across a composer switch (real debounce)", ( expect(store().inputDrafts).toEqual({ __new__: "half a thought" }); }); + // An in-flight upload PINS the editor to the source document: Guard 0 in + // ContentEditor refuses to setContent over an `uploading` node (doing so + // strands the upload's finalize and the file vanishes). So while the upload + // runs, the instance still holds A's document even though the user is on B — + // and the upload's own completion dispatch fires onUpdate on that instance. + // Every write it produces belongs to A, not to wherever the user navigated. + // + // `useUploadGate` does NOT protect this: it gates submit, not session/agent + // navigation (use-chat-controller.ts handleSelectSession has no upload check). + describe("with an upload in flight", () => { + it("keeps the completing upload's markdown in the source draft, never the one switched to", () => { + store().activeSessionId = "session-a"; + store().inputDrafts["session-b"] = "B's own words"; + const { rerender } = render(element()); + + beginUpload("look at this ![](blob:local-preview)"); + + // User clicks session B while the upload is still running. + store().activeSessionId = "session-b"; + rerender(element()); + + // Upload lands: blob → CDN URL, uploading flag cleared, dispatched into + // the same instance. + finishUpload("look at this ![](/api/attachments/att-1/download)"); + act(() => { + vi.advanceTimersByTime(1000); + }); + + // B must never receive A's body or its attachment URL. + expect(store().inputDrafts["session-b"]).toBe("B's own words"); + // The upload result belongs to the draft it was started in. + expect(store().inputDrafts["session-a"]).toBe( + "look at this ![](/api/attachments/att-1/download)", + ); + }); + + it("binds an attachment dropped while the editor is still pinned to the source draft", async () => { + store().activeSessionId = "session-a"; + const onUploadFile = vi.fn(async (_file: File) => makeUpload("att-2", "second.png")); + const { rerender } = render(element({ onUploadFile })); + + // An upload is already in flight, so Guard 0 pins the editor to A's + // document. + beginUpload("first ![](blob:one)"); + store().activeSessionId = "session-b"; + rerender(element({ onUploadFile })); + + // The composer still shows A's document, so a file dropped now lands in + // A's body — its attachment row has to bind to A, or the body and the + // staged attachment end up in different drafts. + await act(async () => { + fireEvent.drop(screen.getByTestId("editor-content"), { + dataTransfer: { files: [new File(["x"], "second.png", { type: "image/png" })] }, + }); + await Promise.resolve(); + }); + + const addAttachment = ( + useChatStore.getState() as unknown as { + addInputDraftAttachment: ReturnType; + } + ).addInputDraftAttachment; + expect(addAttachment).toHaveBeenCalledWith( + "session-a", + expect.objectContaining({ id: "att-2" }), + ); + expect(addAttachment).not.toHaveBeenCalledWith("session-b", expect.anything()); + }); + + it("refuses to send while the composer still holds the source draft's document", () => { + // The upload gate covers most of the pinned window, but not the sliver + // between the upload's final dispatch (uploading node gone, so + // `hasActiveUploads()` is already false) and the re-render that adopts — + // React flushes that update on a scheduler task, so a click can land + // first. Sending then would post A's text into B's session. + store().activeSessionId = "session-a"; + const onSend = vi.fn(); + const { rerender } = render(element({ onSend })); + + beginUpload("A's words ![](blob:one)"); + store().activeSessionId = "session-b"; + rerender(element({ onSend })); + + // Upload's node is gone, but no transaction has re-rendered us yet, so + // the adopt has not run: the editor still holds A. + editorState.uploadingNodes = []; + act(() => { + latestEditorOptions.current?.onUpdate?.({ editor: editorInstance.current }); + }); + + // Mod+Enter, which skips the SubmitButton (and its disabled state) + // entirely — the only way to actually reach handleSend in this window. + act(() => { + capturedExtOptions.current?.onSubmitRef?.current?.(); + }); + + expect(onSend).not.toHaveBeenCalled(); + }); + + it("loads the target draft once the upload guard clears", () => { + store().activeSessionId = "session-a"; + store().inputDrafts["session-b"] = "B's own words"; + const { rerender } = render(element()); + + beginUpload("uploading ![](blob:local-preview)"); + store().activeSessionId = "session-b"; + rerender(element()); + + // Guard 0 blocks the sync while the upload runs — and it CONSUMES the + // defaultValue change (lastDefaultValueRef advances before the guard), so + // nothing re-applies it later on its own. + mockSetContent.mockClear(); + + finishUpload("uploaded ![](/api/attachments/att-1/download)"); + act(() => { + vi.advanceTimersByTime(1000); + }); + + // The composer must end up showing B's draft, not A's document. + expect(mockSetContent).toHaveBeenCalled(); + }); + }); + it("does not strand the last keystrokes when a lazy session create re-keys the draft mid-compose", () => { // First send / first upload in a New Chat flips activeSessionId null → uuid // under a live editor. Those bytes were typed in the New Chat slot, so they diff --git a/packages/views/chat/components/chat-input.test.tsx b/packages/views/chat/components/chat-input.test.tsx index 9f92a0d049..b97074378e 100644 --- a/packages/views/chat/components/chat-input.test.tsx +++ b/packages/views/chat/components/chat-input.test.tsx @@ -119,6 +119,11 @@ vi.mock("../../editor", async () => ({ // the draft-switch flush that depends on it) is covered against the real // ContentEditor in chat-input-draft-isolation.test.tsx. flushPendingUpdate: () => null, + // Same file: the upload-pinned adopt path needs the real Guard 0, which + // this mock has no concept of. Kept so the ref honours the full contract. + adoptContent: (markdown: string) => { + valueRef.current = markdown; + }, })); return (