diff --git a/packages/views/chat/components/chat-input.test.tsx b/packages/views/chat/components/chat-input.test.tsx index b3ab6a6f4..48c24cc1e 100644 --- a/packages/views/chat/components/chat-input.test.tsx +++ b/packages/views/chat/components/chat-input.test.tsx @@ -71,6 +71,7 @@ vi.mock("../../editor", () => ({ clearContent: () => { valueRef.current = ""; }, + cancelPendingUpdate: () => {}, blur: () => {}, focus: () => {}, uploadFile: async (file: File) => { diff --git a/packages/views/chat/components/chat-input.tsx b/packages/views/chat/components/chat-input.tsx index 20b0644f5..5e48d859b 100644 --- a/packages/views/chat/components/chat-input.tsx +++ b/packages/views/chat/components/chat-input.tsx @@ -204,6 +204,14 @@ export function ChatInput({ draftKey: keyAtSend, attachmentCount: activeIds.length, }); + // Freeze the editor→store debounce channel for the duration of this send. + // onSend lazy-creates the session and flips activeSessionId, which changes + // draftKey from `__new__:agent` → the session id. A trailing debounced + // onUpdate firing after that flip would write the draft under the NEW key, + // which clearInputDraft(keyAtSend) — keyed on the pre-send draft — never + // clears; the defaultValue sync would then replay the stranded text back + // into the just-cleared editor. Cancelling here makes the send atomic. + editorRef.current?.cancelPendingUpdate(); setIsSubmitting(true); let accepted: void | boolean; try { diff --git a/packages/views/editor/content-editor.tsx b/packages/views/editor/content-editor.tsx index c709991f4..d80dbcf00 100644 --- a/packages/views/editor/content-editor.tsx +++ b/packages/views/editor/content-editor.tsx @@ -140,6 +140,13 @@ interface ContentEditorProps { interface ContentEditorRef { getMarkdown: () => string; clearContent: () => void; + /** Drop any pending debounced `onUpdate` without flushing it. Used by chat + * at the START of a send so a trailing editor→store write can't fire after + * `activeSessionId` flips and land the draft under the new session key — + * which the pre-send `clearInputDraft(keyAtSend)` would then miss, leaving + * the just-sent text to be replayed back into the cleared editor. Makes a + * send atomic with respect to the draft store. */ + cancelPendingUpdate: () => void; focus: () => void; /** Drop focus from the editor — used by chat after send so the caret * stops competing with the StatusPill / streaming reply for the user's @@ -461,6 +468,27 @@ const ContentEditor = forwardRef( getMarkdown: () => stripBlobUrls(editor?.getMarkdown() ?? ""), clearContent: () => { editor?.commands.clearContent(); + // clearContent dispatches a transaction that fires onUpdate + // synchronously, arming a fresh debounce. Cancel it so the clear + // doesn't trail a "" write under a key the caller didn't intend, and + // reset the dirty baseline so the defaultValue sync effect treats the + // now-empty editor as clean (not as unsaved local edits). + if (debounceRef.current) { + clearTimeout(debounceRef.current); + debounceRef.current = undefined; + } + pendingFlushRef.current = null; + lastEmittedRef.current = ""; + }, + cancelPendingUpdate: () => { + if (debounceRef.current) { + clearTimeout(debounceRef.current); + debounceRef.current = undefined; + } + pendingFlushRef.current = null; + // Align the dirty baseline to what's currently in the editor so the + // dropped write leaves no "unsaved edits" residue for the sync effect. + lastEmittedRef.current = stripBlobUrls(editor?.getMarkdown() ?? "").trimEnd(); }, focus: () => { editor?.commands.focus(); diff --git a/packages/views/issues/components/comment-composers.test.tsx b/packages/views/issues/components/comment-composers.test.tsx index 0121e6816..27b49355c 100644 --- a/packages/views/issues/components/comment-composers.test.tsx +++ b/packages/views/issues/components/comment-composers.test.tsx @@ -8,6 +8,9 @@ import { CommentInput } from "./comment-input"; import { ReplyInput } from "./reply-input"; const uploadWithToast = vi.hoisted(() => vi.fn()); +// Mutable editor state shared with the ContentEditor mock so a test can +// simulate "an upload is still in flight" and assert submit is blocked. +const editorState = vi.hoisted(() => ({ hasActiveUploads: false })); vi.mock("@multica/core/api", () => ({ api: {}, @@ -60,7 +63,7 @@ vi.mock("../../editor", () => ({ valueRef.current = `${valueRef.current}\n${result.url}`.trim(); onUpdate?.(valueRef.current); }, - hasActiveUploads: () => false, + hasActiveUploads: () => editorState.hasActiveUploads, })); return ( @@ -121,6 +124,7 @@ function getSubmitButton(container: HTMLElement): HTMLButtonElement { beforeEach(() => { uploadWithToast.mockReset(); + editorState.hasActiveUploads = false; localStorage.clear(); }); @@ -184,4 +188,32 @@ describe("comment composers", () => { expect(onSubmit).toHaveBeenCalledWith("thread reply", undefined, undefined); }); }); + + it("blocks comment submit while a file is still uploading", async () => { + editorState.hasActiveUploads = true; + const { container, onSubmit } = renderCommentInput(); + + fireEvent.change(screen.getByTestId("editor"), { + target: { value: "comment with pending upload" }, + }); + fireEvent.click(getSubmitButton(container)); + + // The mid-upload submit must be dropped — otherwise the comment sends + // without its attachment bound and the placeholder keeps spinning. + await new Promise((r) => setTimeout(r, 0)); + expect(onSubmit).not.toHaveBeenCalled(); + }); + + it("blocks reply submit while a file is still uploading", async () => { + editorState.hasActiveUploads = true; + const { container, onSubmit } = renderReplyInput(); + + fireEvent.change(screen.getByTestId("editor"), { + target: { value: "reply with pending upload" }, + }); + fireEvent.click(getSubmitButton(container)); + + await new Promise((r) => setTimeout(r, 0)); + expect(onSubmit).not.toHaveBeenCalled(); + }); }); diff --git a/packages/views/issues/components/comment-input.tsx b/packages/views/issues/components/comment-input.tsx index f1793a0b2..91a999333 100644 --- a/packages/views/issues/components/comment-input.tsx +++ b/packages/views/issues/components/comment-input.tsx @@ -38,6 +38,10 @@ function CommentInput({ issueId, onSubmit }: CommentInputProps) { // - the editor's AttachmentDownloadProvider, so file-card Eye buttons can // resolve text/code/markdown previews that require the attachment id. const [pendingAttachments, setPendingAttachments] = useState([]); + // Number of in-flight uploads. Drives the SubmitButton disabled state so it + // greys out the instant an upload starts; handleSubmit ALSO gates on the + // editor's hasActiveUploads() for the Mod+Enter path that bypasses the button. + const [pendingUploads, setPendingUploads] = useState(0); const { uploadWithToast } = useFileUpload(api); const { isDragOver, dropZoneProps } = useFileDropZone({ onDrop: (files) => files.forEach((f) => editorRef.current?.uploadFile(f)), @@ -64,11 +68,16 @@ function CommentInput({ issueId, onSubmit }: CommentInputProps) { }, [draftKey, setDraft]); const handleUpload = useCallback(async (file: File) => { - const result = await uploadWithToast(file, { issueId }); - if (result) { - setPendingAttachments((prev) => [...prev, result]); + setPendingUploads((n) => n + 1); + try { + const result = await uploadWithToast(file, { issueId }); + if (result) { + setPendingAttachments((prev) => [...prev, result]); + } + return result; + } finally { + setPendingUploads((n) => Math.max(0, n - 1)); } - return result; }, [uploadWithToast, issueId]); useEffect(() => { @@ -95,6 +104,12 @@ function CommentInput({ issueId, onSubmit }: CommentInputProps) { const handleSubmit = async () => { const content = editorRef.current?.getMarkdown()?.replace(/(\n\s*)+$/, "").trim(); if (!content || submitting) return; + // Block submit while a file is still uploading. The attachment id isn't in + // pendingAttachments until the upload resolves, so a submit mid-upload drops + // attachment_ids (the file binds to nothing) and leaves the uploading + // placeholder spinning inside an already-sent comment. The SubmitButton is + // disabled via pendingUploads, but Mod+Enter bypasses it so we gate here too. + if (editorRef.current?.hasActiveUploads()) return; // Track every attachment whose stable download URL OR legacy // storage URL is referenced in the markdown body. Both shapes // can appear in the same comment during the MUL-3130 rollout — @@ -165,7 +180,7 @@ function CommentInput({ issueId, onSubmit }: CommentInputProps) { /> 0} loading={submitting} tooltip={`${t(($) => $.comment.send_tooltip)} · ${formatShortcut(modKey, enterKey)}`} /> diff --git a/packages/views/issues/components/reply-input.tsx b/packages/views/issues/components/reply-input.tsx index cfa02434c..d2d9a4006 100644 --- a/packages/views/issues/components/reply-input.tsx +++ b/packages/views/issues/components/reply-input.tsx @@ -66,6 +66,9 @@ function ReplyInput({ // Attachments uploaded in this composer session — see CommentInput for the // rationale (drives both submit-time attachment_ids and editor previews). const [pendingAttachments, setPendingAttachments] = useState([]); + // In-flight upload count — disables the send button while a file is still + // uploading. handleSubmit also gates on hasActiveUploads() for Mod+Enter. + const [pendingUploads, setPendingUploads] = useState(0); const { uploadWithToast } = useFileUpload(api); const { isDragOver, dropZoneProps } = useFileDropZone({ onDrop: (files) => files.forEach((f) => editorRef.current?.uploadFile(f)), @@ -88,11 +91,16 @@ function ReplyInput({ }, [draftKey, setDraft]); const handleUpload = useCallback(async (file: File) => { - const result = await uploadWithToast(file, { issueId }); - if (result) { - setPendingAttachments((prev) => [...prev, result]); + setPendingUploads((n) => n + 1); + try { + const result = await uploadWithToast(file, { issueId }); + if (result) { + setPendingAttachments((prev) => [...prev, result]); + } + return result; + } finally { + setPendingUploads((n) => Math.max(0, n - 1)); } - return result; }, [uploadWithToast, issueId]); useEffect(() => { @@ -119,6 +127,10 @@ function ReplyInput({ const handleSubmit = async () => { const content = editorRef.current?.getMarkdown()?.replace(/(\n\s*)+$/, "").trim(); if (!content || submitting) return; + // Block submit while a file is still uploading — see CommentInput: a + // mid-upload submit drops attachment_ids and strands the spinning + // placeholder in an already-sent reply. Gate here for the Mod+Enter path. + if (editorRef.current?.hasActiveUploads()) return; // Track every attachment whose stable download URL OR legacy // storage URL is referenced in the markdown body. Both shapes // can appear in the same comment during the MUL-3130 rollout. @@ -202,7 +214,7 @@ function ReplyInput({ type="button" variant={isEmpty ? "ghost" : "default"} size="icon-xs" - disabled={isEmpty || submitting} + disabled={isEmpty || submitting || pendingUploads > 0} onClick={handleSubmit} > {submitting ? (