mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-05 13:29:44 +02:00
fix(composer): make chat send atomic and gate comment/reply submit on uploads
Two independent composer bugs surfaced on slow/local backends. Chat: after sending in a new chat, the just-sent text could reappear in the input. Root cause is the dual source of truth (Tiptap doc + the inputDrafts store) bridged by a debounced onUpdate. onSend lazy-creates the session and flips activeSessionId, changing draftKey from `__new__:agent` to the session id. A trailing debounced write firing after the flip lands the draft under the NEW key, which clearInputDraft(keyAtSend) — keyed on the pre-send draft — never clears; the defaultValue sync then replays the stranded text back into the cleared editor. Fix freezes the editor→store channel at send start via a new ContentEditorRef.cancelPendingUpdate(), making the send atomic, and has clearContent() drop its own trailing debounce + reset the dirty baseline. The store→editor sync is kept (it is load-bearing for same-agent session switching, which does not remount the editor). Comment/reply: submitting while a file is still uploading sent the body without binding the attachment (its id is not in pendingAttachments until the upload resolves) and left the uploading placeholder spinning in an already-sent comment. Mirror chat-input's guard: gate handleSubmit on hasActiveUploads() (covers the Mod+Enter path) and disable the send button while pendingUploads > 0. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -71,6 +71,7 @@ vi.mock("../../editor", () => ({
|
||||
clearContent: () => {
|
||||
valueRef.current = "";
|
||||
},
|
||||
cancelPendingUpdate: () => {},
|
||||
blur: () => {},
|
||||
focus: () => {},
|
||||
uploadFile: async (file: File) => {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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<ContentEditorRef, ContentEditorProps>(
|
||||
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();
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<Attachment[]>([]);
|
||||
// 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) {
|
||||
/>
|
||||
<SubmitButton
|
||||
onClick={handleSubmit}
|
||||
disabled={isEmpty}
|
||||
disabled={isEmpty || submitting || pendingUploads > 0}
|
||||
loading={submitting}
|
||||
tooltip={`${t(($) => $.comment.send_tooltip)} · ${formatShortcut(modKey, enterKey)}`}
|
||||
/>
|
||||
|
||||
@@ -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<Attachment[]>([]);
|
||||
// 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 ? (
|
||||
|
||||
Reference in New Issue
Block a user