MUL-3254: flush issue description edits on close (#4082)

* fix: flush issue description editor on close

Co-authored-by: multica-agent <github@multica.ai>

* fix: make unmount flush opt-in via flushPendingOnUnmount

The unconditional unmount flush re-emitted discarded content into
composers that clear their draft and then unmount (comment edit cancel,
create-issue / feedback submit), resurrecting the cleared draft.

- Add flushPendingOnUnmount prop (default false); only the issue-detail
  description editor opts in.
- Cache the pending markdown in a ref at onUpdate time and emit that
  cached copy on unmount, instead of reading the editor instance during
  teardown.
- Regression tests: default drops the pending update on unmount, opt-in
  flush emits the cached value even when the editor is already
  destroyed, no double-emit after the debounce fired, and issue-detail
  pins the opt-in wiring.

Co-authored-by: multica-agent <github@multica.ai>

---------

Co-authored-by: J <j@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>

packages/views/editor/content-editor.test.tsx

@@ -1,4 +1,4 @@
-import { describe, it, expect, vi, beforeEach } from "vitest";
+import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
 import { act, fireEvent, render, screen } from "@testing-library/react";
 import type { Attachment } from "@multica/core/types";
 import type { UploadResult } from "@multica/core/hooks/use-file-upload";
@@ -57,9 +57,16 @@ vi.mock("./attachment-download-context", () => ({
 
 const editorRef = vi.hoisted<{ current: unknown }>(() => ({ current: null }));
 const onCreateFired = vi.hoisted(() => ({ value: false }));
+const latestEditorOptions = vi.hoisted<{
+  current?: { onUpdate?: (args: { editor: unknown }) => void };
+}>(() => ({}));
 
 vi.mock("@tiptap/react", () => ({
-  useEditor: (options: { onCreate?: (args: { editor: unknown }) => void }) => {
+  useEditor: (options: {
+    onCreate?: (args: { editor: unknown }) => void;
+    onUpdate?: (args: { editor: unknown }) => void;
+  }) => {
+    latestEditorOptions.current = options;
     if (!editorRef.current) {
       editorRef.current = {
         get isFocused() {
@@ -104,9 +111,14 @@ describe("ContentEditor", () => {
     editorState.markdown = "";
     editorRef.current = null;
     onCreateFired.value = false;
+    latestEditorOptions.current = undefined;
     providerProps.attachments = undefined;
   });
 
+  afterEach(() => {
+    vi.useRealTimers();
+  });
+
   it("focuses the editor when clicking the empty container area", () => {
     render(<ContentEditor placeholder="Add description..." />);
 
@@ -210,6 +222,98 @@ describe("ContentEditor", () => {
 
     expect(mockSetContent).not.toHaveBeenCalled();
   });
+
+  it("flushes a pending debounced update on unmount when flushPendingOnUnmount is set", () => {
+    vi.useFakeTimers();
+    const onUpdate = vi.fn();
+    editorState.markdown = "old content";
+    const { unmount } = render(
+      <ContentEditor
+        defaultValue="old content"
+        onUpdate={onUpdate}
+        debounceMs={1500}
+        flushPendingOnUnmount
+      />,
+    );
+
+    editorState.markdown = "old content\n\n![shot](/api/attachments/att-1/download)";
+    act(() => {
+      latestEditorOptions.current?.onUpdate?.({ editor: editorRef.current });
+    });
+
+    expect(onUpdate).not.toHaveBeenCalled();
+
+    // The flush must emit the copy cached at onUpdate time — by cleanup time
+    // Tiptap may already have torn the instance down, so reading the editor
+    // during unmount is not an option.
+    editorState.isDestroyed = true;
+    editorState.markdown = "";
+
+    unmount();
+
+    expect(onUpdate).toHaveBeenCalledTimes(1);
+    expect(onUpdate).toHaveBeenCalledWith(
+      "old content\n\n![shot](/api/attachments/att-1/download)",
+    );
+    act(() => {
+      vi.advanceTimersByTime(1500);
+    });
+    expect(onUpdate).toHaveBeenCalledTimes(1);
+  });
+
+  it("drops a pending debounced update on unmount by default", () => {
+    // Regression guard for draft resurrection: composers like comment edit
+    // cancel `clearDraft()` and then unmount this editor. A default unmount
+    // flush would re-emit the discarded markdown into onUpdate, which writes
+    // it straight back into the draft store.
+    vi.useFakeTimers();
+    const onUpdate = vi.fn();
+    editorState.markdown = "edit draft the user cancelled";
+    const { unmount } = render(
+      <ContentEditor
+        defaultValue=""
+        onUpdate={onUpdate}
+        debounceMs={300}
+      />,
+    );
+
+    act(() => {
+      latestEditorOptions.current?.onUpdate?.({ editor: editorRef.current });
+    });
+    expect(onUpdate).not.toHaveBeenCalled();
+
+    unmount();
+
+    expect(onUpdate).not.toHaveBeenCalled();
+    act(() => {
+      vi.advanceTimersByTime(300);
+    });
+    expect(onUpdate).not.toHaveBeenCalled();
+  });
+
+  it("does not re-emit on unmount when the debounce already fired", () => {
+    vi.useFakeTimers();
+    const onUpdate = vi.fn();
+    const { unmount } = render(
+      <ContentEditor
+        defaultValue=""
+        onUpdate={onUpdate}
+        debounceMs={1500}
+        flushPendingOnUnmount
+      />,
+    );
+
+    editorState.markdown = "typed content";
+    act(() => {
+      latestEditorOptions.current?.onUpdate?.({ editor: editorRef.current });
+      vi.advanceTimersByTime(1500);
+    });
+    expect(onUpdate).toHaveBeenCalledTimes(1);
+
+    unmount();
+
+    expect(onUpdate).toHaveBeenCalledTimes(1);
+  });
 });
 
 function makeAttachment(id: string, overrides: Partial<Attachment> = {}): Attachment {

packages/views/editor/content-editor.tsx

@@ -124,6 +124,17 @@ interface ContentEditorProps {
    * available (NodeView buttons fall back to opening the raw URL).
    */
   attachments?: Attachment[];
+  /**
+   * Flush a pending debounced `onUpdate` when the editor unmounts instead of
+   * dropping it. Default false ON PURPOSE: most composers clear their draft
+   * and then unmount (comment edit cancel, create-issue / feedback submit),
+   * and a flush there would hand the discarded content right back to
+   * `onUpdate`, resurrecting the cleared draft. Opt in only where closing
+   * means "keep what the user last saw" — e.g. the issue-detail description
+   * editor, whose 1500ms debounce would otherwise drop a paste made just
+   * before the modal closes.
+   */
+  flushPendingOnUnmount?: boolean;
 }
 
 interface ContentEditorRef {
@@ -163,10 +174,16 @@ const ContentEditor = forwardRef<ContentEditorRef, ContentEditorProps>(
       enableSlashCommands = false,
       slashCommandMode = "skill",
       attachments,
+      flushPendingOnUnmount = false,
     },
     ref,
   ) {
     const debounceRef = useRef<ReturnType<typeof setTimeout>>(undefined);
+    const flushPendingOnUnmountRef = useRef(flushPendingOnUnmount);
+    // Markdown serialized at `onUpdate` time, awaiting its debounce fire. The
+    // unmount flush emits this cached copy — it runs mid-teardown and can't
+    // assume the editor instance is still readable.
+    const pendingFlushRef = useRef<string | null>(null);
     const onUpdateRef = useRef(onUpdate);
     const onSubmitRef = useRef(onSubmit);
     const onBlurRef = useRef(onBlur);
@@ -249,6 +266,7 @@ const ContentEditor = forwardRef<ContentEditorRef, ContentEditorProps>(
     onBlurRef.current = onBlur;
     onUploadFileRef.current = wrappedOnUploadFile;
     mentionContextItemsRef.current = mentionContextItems ?? [];
+    flushPendingOnUnmountRef.current = flushPendingOnUnmount;
 
     const queryClient = useQueryClient();
 
@@ -303,8 +321,13 @@ const ContentEditor = forwardRef<ContentEditorRef, ContentEditorProps>(
       }),
       onUpdate: ({ editor: ed }) => {
         if (!onUpdateRef.current) return;
+        if (flushPendingOnUnmountRef.current) {
+          pendingFlushRef.current = stripBlobUrls(ed.getMarkdown()).trimEnd();
+        }
         if (debounceRef.current) clearTimeout(debounceRef.current);
         debounceRef.current = setTimeout(() => {
+          debounceRef.current = undefined;
+          pendingFlushRef.current = null;
           const md = stripBlobUrls(ed.getMarkdown()).trimEnd();
           if (md === lastEmittedRef.current) return;
           lastEmittedRef.current = md;
@@ -336,10 +359,21 @@ const ContentEditor = forwardRef<ContentEditorRef, ContentEditorProps>(
       },
     });
 
-    // Cleanup debounce on unmount
+    // Cleanup on unmount. A pending debounced update is DROPPED by default,
+    // not flushed — see the `flushPendingOnUnmount` prop doc for why. When the
+    // owner opted in, emit the markdown cached at `onUpdate` time so a long
+    // debounce can't swallow the last edit when the surrounding modal closes.
     useEffect(() => {
       return () => {
-        if (debounceRef.current) clearTimeout(debounceRef.current);
+        if (!debounceRef.current) return;
+        clearTimeout(debounceRef.current);
+        debounceRef.current = undefined;
+        if (!flushPendingOnUnmountRef.current) return;
+        const pending = pendingFlushRef.current;
+        pendingFlushRef.current = null;
+        if (pending === null || pending === lastEmittedRef.current) return;
+        lastEmittedRef.current = pending;
+        onUpdateRef.current?.(pending);
       };
     }, []);
 

packages/views/issues/components/issue-detail.test.tsx

@@ -129,7 +129,7 @@ vi.mock("../../editor", () => ({
     <div data-testid="readonly-content">{content}</div>
   ),
   ContentEditor: forwardRef(function MockContentEditor(
-    { defaultValue, onUpdate, placeholder }: any,
+    { defaultValue, onUpdate, placeholder, flushPendingOnUnmount }: any,
     ref: any,
   ) {
     const valueRef = useRef(defaultValue || "");
@@ -150,6 +150,7 @@ vi.mock("../../editor", () => ({
         }}
         placeholder={placeholder}
         data-testid="rich-text-editor"
+        data-flush-on-unmount={flushPendingOnUnmount ? "true" : undefined}
       />
     );
   }),
@@ -535,6 +536,19 @@ describe("IssueDetail (shared)", () => {
     expect(screen.getByDisplayValue("Add JWT auth to the backend")).toBeInTheDocument();
   });
 
+  it("opts the description editor into the unmount flush", async () => {
+    // Closing the issue modal must save the description the user last saw —
+    // ContentEditor drops pending debounced updates on unmount by default
+    // (so cancelled comment drafts aren't resurrected), and only this
+    // explicit opt-in keeps a paste-then-close from losing the image
+    // markdown and its attachment_ids bind (MUL-3254). The flush behavior
+    // itself is covered in content-editor.test.tsx; this pins the wiring.
+    renderIssueDetail();
+
+    const description = await screen.findByDisplayValue("Add JWT auth to the backend");
+    expect(description).toHaveAttribute("data-flush-on-unmount", "true");
+  });
+
   it("renders the issue title leaf as a link to the issue detail page", async () => {
     renderIssueDetail();
 

packages/views/issues/components/issue-detail.tsx

@@ -1198,18 +1198,30 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr
   // `attachment_ids` on the next description save. Drives editor previews
   // so text/code attachments show an Eye before the bind round-trips.
   const [descPendingAttachments, setDescPendingAttachments] = useState<Attachment[]>([]);
+  const descPendingAttachmentsRef = useRef<Attachment[]>([]);
   const descEditorAttachments = descPendingAttachments.length > 0
     ? [...(issueAttachments ?? []), ...descPendingAttachments]
     : issueAttachments;
   const handleDescriptionUpload = useCallback(
     async (file: File) => {
       const result = await uploadWithToast(file);
-      if (result) setDescPendingAttachments((prev) => [...prev, result]);
+      if (result) {
+        descPendingAttachmentsRef.current = [
+          ...descPendingAttachmentsRef.current,
+          result,
+        ];
+        setDescPendingAttachments(descPendingAttachmentsRef.current);
+      }
       return result;
     },
     [uploadWithToast],
   );
 
+  useEffect(() => {
+    descPendingAttachmentsRef.current = [];
+    setDescPendingAttachments([]);
+  }, [id]);
+
   // Shared issue actions (mutations, pin, copy-link, modal dispatch, etc.).
   // Called before the `if (!issue)` early return so hook order stays stable.
   const actions = useIssueActions(issue);
@@ -1862,13 +1874,17 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr
                 // webview) — while still working on web via the cookie/proxy.
                 // This mirrors the comment/reply/chat composers, which already
                 // bind via `contentReferencesAttachment` (MUL-3130 / MUL-3192).
-                const ids = descPendingAttachments
+                const ids = descPendingAttachmentsRef.current
                   .filter((a) => contentReferencesAttachment(md, a))
                   .map((a) => a.id);
                 handleUpdateField({ description: md, attachment_ids: ids.length > 0 ? ids : undefined });
               }}
               onUploadFile={handleDescriptionUpload}
               debounceMs={1500}
+              // Closing the issue modal must save what the user last saw —
+              // without the flush, a paste followed by a quick close loses
+              // the image markdown and its attachment_ids bind (MUL-3254).
+              flushPendingOnUnmount
               currentIssueId={id}
               attachments={descEditorAttachments}
             />