🤖 fix: keep an open edit out of the shared draft

src/browser/components/ChatPane/ChatPane.tsx

@@ -873,7 +873,7 @@ const ChatPaneContent: React.FC<ChatPaneContentProps> = (props) => {
   const chatInputAPI = useRef<ChatInputAPI | null>(null);
 
   const handleQuoteText = useCallback((quotedText: string) => {
-    chatInputAPI.current?.appendText(quotedText);
+    chatInputAPI.current?.appendToVisibleInput(quotedText);
     chatInputAPI.current?.focus();
   }, []);
 
@@ -939,6 +939,8 @@ const ChatPaneContent: React.FC<ChatPaneContentProps> = (props) => {
     async (queuedMessage: QueuedMessageData) => {
       const inputApi = chatInputAPI.current;
       if (!inputApi) return;
+      // A visible edit would hide the restored message behind it: keep it queued instead.
+      if (inputApi.isEditVisible()) return;
 
       await api?.workspace.clearQueue({ workspaceId });
       inputApi.restoreDraft(normalizeQueuedMessage(queuedMessage), {

src/browser/features/ChatInput/types.ts

@@ -24,6 +24,10 @@ export interface ChatInputAPI {
   restoreDraft: (pending: PendingUserMessage, options?: { retainedSendIds?: string[] }) => void;
   appendText: (text: string) => void;
   prependText: (text: string) => void;
+  /** Append where the composer shows it: the visible edit, else the draft (quotes, inserts). */
+  appendToVisibleInput: (text: string) => void;
+  /** Whether the composer shows an open edit; inserts that cannot join it are refused then. */
+  isEditVisible: () => boolean;
 }
 
 export interface WorkspaceCreatedOptions {

src/browser/features/ChatInput/useComposerAttachments.ts

@@ -21,6 +21,8 @@ interface UseComposerAttachmentsOptions {
     value: ChatAttachment[] | ((previous: ChatAttachment[]) => ChatAttachment[])
   ) => void;
   editingMessage: boolean;
+  /** Live check, read when a file lands: the composer may show an edit by then. */
+  isEditVisible: () => boolean;
   pushToast: PushToast;
 }
 function getBaseMediaType(mediaType: string): string {
@@ -98,7 +100,9 @@ export function useComposerAttachments(options: UseComposerAttachmentsOptions) {
       )
     ).then((outcomes) => {
       const successes = outcomes.flatMap((outcome) => (outcome.ok ? outcome.attachments : []));
-      if (successes.length > 0) {
+      // An add that started before an edit opened lands in neither the edit nor the draft.
+      if (successes.length > 0 && options.isEditVisible()) rejectEditAttachment();
+      else if (successes.length > 0) {
         setAttachments((previous) => [...previous, ...successes]);
         showResizeToast(successes);
       }

src/browser/features/ChatInput/useComposerDraft.test.ts

@@ -78,4 +78,32 @@ describe("useComposerDraft", () => {
 
     expect(result.current.attachments.map(({ id }) => id)).toEqual(["a", "b"]);
   });
+
+  // T18: an in-flight send or command owns a detached snapshot; it never touches the live edit.
+  test("a snapshot append leaves the edit buffer alone; consuming the buffer appends it once", () => {
+    const workspaceId = "ws-edit-buffer";
+    const { result } = renderDraft(workspaceId);
+    act(() => {
+      result.current.setInput("unsent");
+      result.current.setAttachments([attachment("unsent")]);
+      result.current.beginEditDraft({ id: "edit-1", text: "edit", attachments: [attachment("e")] });
+    });
+    act(() => {
+      result.current.appendSnapshotToDraft({ text: "snapshot", attachments: [attachment("s")] });
+    });
+
+    const view = () => getDraftStore().getView({ kind: "workspace", workspaceId });
+    expect(view().text).toBe("unsent\n\nsnapshot");
+    expect(view().attachments.map(({ id }) => id)).toEqual(["unsent", "s"]);
+    expect(result.current.editDraft?.text).toBe("edit");
+    expect(result.current.editDraft?.attachments.map(({ id }) => id)).toEqual(["e"]);
+
+    act(() => {
+      result.current.consumeEditDraftIntoDraft();
+      result.current.consumeEditDraftIntoDraft();
+    });
+    expect(view().text).toBe("unsent\n\nsnapshot\n\nedit");
+    expect(view().attachments.map(({ id }) => id)).toEqual(["unsent", "s", "e"]);
+    expect(result.current.editDraft).toBeNull();
+  });
 });

src/browser/features/ChatInput/useComposerDraft.ts

@@ -7,6 +7,7 @@ import {
 } from "@/browser/stores/DraftStore";
 import type { ReviewNoteDataForDisplay } from "@/common/types/message";
 import type { Review } from "@/common/types/review";
+import { joinDraftText } from "@/common/utils/composerDraftText";
 import { DRAFT_ID_PATTERN } from "@/constants/drafts";
 import type { ChatAttachment } from "./ChatAttachments";
 import type { Toast } from "./ChatInputToast";
@@ -43,6 +44,25 @@ export function getComposerDraftScope(options: {
     : defaultCreationDraftScope(options.creationProjectPath);
 }
 
+/** An open edit's text and files. They live only in the composer that opened the edit. */
+export interface EditDraft {
+  id: string;
+  text: string;
+  attachments: ChatAttachment[];
+}
+
+/** Appends an edit's text after the draft in `scope`, and its files whose ids are missing. */
+function appendToStoredDraft(scope: DraftStoreScope, snapshot: Omit<EditDraft, "id">) {
+  const store = getDraftStore();
+  if (snapshot.text.trim())
+    store.setText(scope, (current) => joinDraftText(current, snapshot.text));
+  if (snapshot.attachments.length === 0) return;
+  store.setAttachments(scope, (current) => {
+    const ids = new Set(current.map(({ id }) => id));
+    return [...current, ...snapshot.attachments.filter(({ id }) => !ids.has(id))];
+  });
+}
+
 export function useComposerDraft(options: UseComposerDraftOptions) {
   const { attachedReviews, pushToast } = options;
   const draftStore = getDraftStore();
@@ -61,6 +81,17 @@ export function useComposerDraft(options: UseComposerDraftOptions) {
   const setAttachments = (
     value: ChatAttachment[] | ((previous: ChatAttachment[]) => ChatAttachment[])
   ) => draftStore.setAttachments(draftScope, value);
+  // The open edit's buffer: an edit never reaches the shared, persisted draft, so a reload or a
+  // second window keeps the unsent draft (#5672, #5571). The ref is the live value.
+  const [editDraft, setEditDraftState] = useState<EditDraft | null>(null);
+  const editDraftRef = useRef<EditDraft | null>(null);
+  const writeEditDraft = (next: EditDraft | null) => {
+    editDraftRef.current = next;
+    setEditDraftState(next);
+  };
+  const updateEditDraft = (update: (current: EditDraft) => EditDraft) => {
+    if (editDraftRef.current) writeEditDraft(update(editDraftRef.current));
+  };
   const pushToastRef = useRef(pushToast);
   pushToastRef.current = pushToast;
   const { variant, workspaceId, creationProjectPath, pendingDraftId } = options;
@@ -86,6 +117,9 @@ export function useComposerDraft(options: UseComposerDraftOptions) {
     });
     return () => {
       unsubscribeSaveErrors();
+      // A switch or transcript-only unmounts the composer with its edit open: no text is lost.
+      if (editDraftRef.current) appendToStoredDraft(scope, editDraftRef.current);
+      editDraftRef.current = null;
       // Leaving this draft (workspace switch, route change, unmount): write it now rather than
       // after the debounce. A failure keeps the change in the store, which retries it.
       getDraftStore()
@@ -140,8 +174,32 @@ export function useComposerDraft(options: UseComposerDraftOptions) {
     setInput(next.text);
     setAttachments(next.attachments);
   };
+  // The only edit operations that write the normal draft: after it, once.
+  const consumeEditDraftIntoDraft = () => {
+    if (editDraftRef.current) appendToStoredDraft(draftScope, editDraftRef.current);
+    writeEditDraft(null);
+  };
   return {
     draftScope,
+    editDraft,
+    getEditDraft: () => editDraftRef.current,
+    beginEditDraft: writeEditDraft,
+    setEditText: (value: string | ((previous: string) => string)) =>
+      updateEditDraft((current) => ({
+        ...current,
+        text: typeof value === "function" ? value(current.text) : value,
+      })),
+    setEditAttachments: (
+      value: ChatAttachment[] | ((previous: ChatAttachment[]) => ChatAttachment[])
+    ) =>
+      updateEditDraft((current) => ({
+        ...current,
+        attachments: typeof value === "function" ? value(current.attachments) : value,
+      })),
+    dropEditDraft: () => writeEditDraft(null),
+    consumeEditDraftIntoDraft,
+    appendSnapshotToDraft: (snapshot: Omit<EditDraft, "id">) =>
+      appendToStoredDraft(draftScope, snapshot),
     input,
     setInput,
     payloadsLoaded: draft.payloadsLoaded,

src/browser/hooks/useAIViewKeybinds.test.tsx

@@ -315,6 +315,8 @@ describe("useAIViewKeybinds", () => {
         restoreDraft: () => undefined,
         appendText: () => undefined,
         prependText: () => undefined,
+        appendToVisibleInput: () => undefined,
+        isEditVisible: () => false,
       },
     };
     renderUseAIViewKeybinds({

tests/bugbash/repros/knownFailureReloadDuringEdit.e2e.ts

@@ -1,5 +1,3 @@
-// Known failure, open issue #5672: a reload during a message edit leaves the edit text in the
-// composer as the new-message draft, and the unsent draft is lost. Fails until #5672 is fixed.
 import { test } from "@e2e-dev/web";
 import { expect } from "e2e";
 import { openPlayground, sendMessageForEdit, WORKSPACE_TITLE } from "./helpers";
@@ -9,7 +7,7 @@ const messageText = () => `5672 message to edit ${Date.now()}`;
 
 test(
   "a reload during an edit keeps the unsent draft",
-  { tags: ["bugbash", "known-failure", "5672"] },
+  { tags: ["bugbash", "5672"] },
   async ({ app, screen, browser }) => {
     await openPlayground(app, screen, browser);
     const MESSAGE = messageText();

tests/ui/chat/composerDraftsFormalRepro.test.ts

@@ -22,7 +22,7 @@ jest.mock("lottie-react", () => ({
   __esModule: true,
   default: () => null,
 }));
-import { fireEvent, waitFor } from "@testing-library/react";
+import { waitFor } from "@testing-library/react";
 import * as fs from "fs/promises";
 import * as path from "path";
 
@@ -199,142 +199,7 @@ describe("formal/composer-drafts: composer text across a failed send", () => {
   }, 120_000);
 });
 
-/**
- * #5501: an edit send (the one send kind without a pending-send entry) still clears the composer
- * and puts what it took back when it fails. Edits write the shared workspace draft (#5571), so
- * another window that still holds the edit can save it again while the send is in flight.
- */
-describe("formal/composer-drafts: a failed edit's put-back", () => {
-  beforeAll(async () => {
-    await preloadTestModules();
-  });
-
-  async function startEdit(app: AppHarness, scope: DraftScope) {
-    await app.chat.send("first message");
-    await app.chat.expectTranscriptContains("Mock response: first message", WAIT.timeout);
-    await app.chat.expectStreamComplete();
-    const editButton = await waitFor(() => {
-      const button = app.view.container.querySelector<HTMLElement>('button[aria-label="Edit"]');
-      if (!button) throw new Error("Edit button not found");
-      return button;
-    }, WAIT);
-    fireEvent.click(editButton);
-    const textarea = await waitFor(() => {
-      const element = app.view.container.querySelector<HTMLTextAreaElement>(
-        'textarea[aria-label="Edit message"]'
-      );
-      if (!element) throw new Error("Edit textarea not found");
-      expect(element.value).toBe("first message");
-      return element;
-    }, WAIT);
-    getDraftStore().setText(scope, "edited message");
-    await waitFor(() => expect(textarea.value).toBe("edited message"), WAIT);
-    return textarea;
-  }
-
-  async function expectRefused(app: AppHarness) {
-    await waitFor(
-      () => expect(app.view.container.textContent ?? "").toContain("formal repro: send refused"),
-      WAIT
-    );
-  }
-
-  test("does not show the edit's text twice when another window saved it meanwhile", async () => {
-    const app = await createAppHarness({ branchPrefix: "formal-edit-restore-dup" });
-    const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId };
-    const textarea = await startEdit(app, scope).catch(async (error: unknown) => {
-      await app.dispose();
-      throw error;
-    });
-    const held = holdSendReplies(app, () => refused());
-    try {
-      fireEvent.keyDown(textarea, { key: "Enter" });
-      await waitFor(() => expect(held.spy).toHaveBeenCalledTimes(1), WAIT);
-      await waitFor(() => expect(getDraftStore().getView(scope).text).toBe(""), WAIT);
-      await getDraftStore().flush(scope);
-
-      // A second window that still held the edit saves it again, with text typed after it.
-      const saved = "edited message\n\ntyped in another window";
-      await app.env.services.draftService.update({ scope, text: saved });
-      await waitFor(() => expect(getDraftStore().getView(scope).text).toBe(saved), WAIT);
-
-      held.release();
-      await expectRefused(app);
-      // Target assertion: the text the composer already holds is not put back a second time,
-      // and the other window's text stays.
-      expect(getDraftStore().getView(scope).text).toBe(saved);
-      await getDraftStore().flush(scope);
-      expect((await app.env.services.draftService.get(scope)).text).toBe(saved);
-    } finally {
-      await held.settle();
-      held.spy.mockRestore();
-      await app.dispose();
-    }
-  }, 120_000);
-
-  test("keeps the staged copy of an attachment another window saved again as pending", async () => {
-    const app = await createAppHarness({ branchPrefix: "formal-edit-restore-staged" });
-    const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId };
-    const textarea = await startEdit(app, scope).catch(async (error: unknown) => {
-      await app.dispose();
-      throw error;
-    });
-    const held = holdSendReplies(app, () => refused());
-    try {
-      const pendingFile = {
-        kind: "pending-file" as const,
-        id: "file-1",
-        mediaType: "text/plain",
-        filename: "notes.txt",
-        sizeBytes: 2,
-        dataBase64: "aGk=",
-      };
-      getDraftStore().setAttachments(scope, [pendingFile]);
-      await waitFor(() => expect(getDraftStore().getView(scope).attachments).toHaveLength(1), WAIT);
-      fireEvent.keyDown(textarea, { key: "Enter" });
-      await waitFor(() => expect(held.spy).toHaveBeenCalledTimes(1), WAIT);
-      // The send staged the file under the same id, then took it out of the composer.
-      await waitFor(() => expect(getDraftStore().getView(scope).attachments).toEqual([]), WAIT);
-      await getDraftStore().flush(scope);
-
-      // A second window that still held the pending version saves it again, with a new file.
-      const newFile = {
-        kind: "provider" as const,
-        id: "file-2",
-        url: "data:text/plain;base64,bmV3",
-        mediaType: "text/plain",
-        filename: "new.txt",
-      };
-      await app.env.services.draftService.update({ scope, attachments: [pendingFile, newFile] });
-      await waitFor(
-        () =>
-          expect(
-            getDraftStore()
-              .getView(scope)
-              .attachments.map(({ kind }) => kind)
-          ).toEqual(["pending-file", "provider"]),
-        WAIT
-      );
-
-      held.release();
-      await expectRefused(app);
-      // Target assertion: the refused send puts back what it sent (the staged file), so a retry
-      // does not stage the file again and orphan the first copy. The new file stays.
-      const restored = getDraftStore().getView(scope).attachments;
-      expect(restored.map(({ id, kind }) => `${id}:${kind}`)).toEqual([
-        "file-1:staged",
-        "file-2:provider",
-      ]);
-      await getDraftStore().flush(scope);
-      const savedAttachments = (await app.env.services.draftService.get(scope)).attachments;
-      expect(savedAttachments.map(({ id, kind }) => `${id}:${kind}`)).toEqual([
-        "file-1:staged",
-        "file-2:provider",
-      ]);
-    } finally {
-      await held.settle();
-      held.spy.mockRestore();
-      await app.dispose();
-    }
-  }, 120_000);
-});
+// #5501's edit put-back repros (another window saving the edit again while its send was in
+// flight) cannot happen any more: an open edit never reaches the shared draft. Their flows are
+// covered in editKeepsUnsentDraft.test.ts ("a second window keeps the unsent draft while the
+// first edits" and "a refused edit send reopens the edit with its text and file").