diff --git a/src/browser/components/ChatPane/ChatPane.tsx b/src/browser/components/ChatPane/ChatPane.tsx index ec0067eb19..8b1eb75700 100644 --- a/src/browser/components/ChatPane/ChatPane.tsx +++ b/src/browser/components/ChatPane/ChatPane.tsx @@ -1354,6 +1354,9 @@ const ChatPaneContent: React.FC = (props) => { // Must be before early return to satisfy React Hooks rules useEffect(() => { if (!workspaceState || !editingMessage) return; + // A replay of this workspace empties its rows first, while ChatPane stays mounted: only a + // caught-up transcript can show that the edited row is gone (#5808). + if (!workspaceState.isTranscriptCaughtUp) return; // Conflict recovery re-reads the transcript (a full replay empties the aggregator first, // a pre-window range discards cached pages); the refresh outcome decides whether the // edited row is gone, not the transient absence of its row. @@ -1374,6 +1377,9 @@ const ChatPaneContent: React.FC = (props) => { )?.historyId; if (!editCutoffHistoryId) { + // A windowed replay can leave the edited row in older history, unloaded: that is not a + // deletion, so the edit stays (#5808). Its send still checks the rows (precondition). + if (workspaceState.hasOlderHistory) return; // Message was replaced or deleted - clear editing state setEditingMessage(undefined); } diff --git a/src/browser/features/ChatInput/index.tsx b/src/browser/features/ChatInput/index.tsx index 47857198df..f8ecbecef4 100644 --- a/src/browser/features/ChatInput/index.tsx +++ b/src/browser/features/ChatInput/index.tsx @@ -309,14 +309,18 @@ function pendingChatAttachments( return [...providerAttachments, ...stagedAttachments]; } -/** One edit, from entering edit mode until it is cancelled or its send is accepted (#5226). */ +/** + * One edit, from entering edit mode until it is cancelled or its send is accepted (#5226). Its + * text lives in the composer's memory-only edit buffer (useComposerDraft), so the unsent draft + * is never replaced and needs no snapshot (#5672, #5571). + */ interface EditSession { id: string; - /** The unsent draft from before the edit, restored when the edit ends. */ - preEditDraft: { text: string; attachments: ChatAttachment[] }; preEditReviews: ReviewNoteDataForDisplay[] | null; /** Its draft was given back (cancel, or accepted send); it restores nothing again. */ settled: boolean; + /** An edit send for it has not returned yet: that send settles it, not releaseEndedEdit. */ + sendInFlight: boolean; } const ChatInputInner: React.FC = (props) => { @@ -506,13 +510,15 @@ const ChatInputInner: React.FC = (props) => { workspaceId, creationProjectPath: creationParentProjectPath, pendingDraftId: variant === "creation" ? (props.pendingDraftId ?? undefined) : undefined, + editMessageId: editingMessage?.id, attachedReviews: variant === "workspace" ? (props.attachedReviews ?? []) : [], pushToast, }); const { input, setInput, attachments, setAttachments, draftReviews, setDraftReviews } = draft; - const { getDraft, setDraft } = draft; + const { getDraft, setDraft, getLiveText, beginEditDraft, endEditDraft, updateEditDraft } = draft; const { reviewOverrideActive, reviewData, reviewIdsForCheck, reviewPanelItems } = draft; const { removeDraftReview, updateDraftReviewNote, draftScope, latestInputValueRef } = draft; + const { draftReviewsRef } = draft; const { processingAttachmentCount, handlePaste, @@ -536,9 +542,14 @@ const ChatInputInner: React.FC = (props) => { // Creation sends can resolve after navigation; guard draft clears on unmounted inputs. const isMountedRef = useRef(true); + const releaseEndedEditRef = useRef<() => void>(() => undefined); useEffect(() => { + // Set here too: StrictMode runs this cleanup once at mount, and the composer lives on. + isMountedRef.current = true; return () => { isMountedRef.current = false; + // An unmount (workspace switch, transcript-only) ends the edit (#5808). + releaseEndedEditRef.current(); }; }, []); const inputRef = useRef(null); @@ -1246,16 +1257,12 @@ const ChatInputInner: React.FC = (props) => { // transcript when the accepted edit replaces it (possibly before the send returns), and that // is not a cancel. const editSessionRef = useRef(null); - // Live review override for completions that settle after the render they started in. - const draftReviewsRef = useRef(draftReviews); - useLayoutEffect(() => { - draftReviewsRef.current = draftReviews; - }); const restorePreEditDraft = () => { const session = editSessionRef.current; if (!session || session.settled || session.id !== editingMessageIdRef.current) return; session.settled = true; - setDraft(session.preEditDraft); + // The edit text is dropped; the composer shows the unsent draft again. + endEditDraft(); setDraftReviews(session.preEditReviews); }; @@ -1270,13 +1277,17 @@ const ChatInputInner: React.FC = (props) => { dropEditReviews = false ): boolean => { if (!session || session.settled) return false; - session.settled = true; - const { preEditDraft, preEditReviews } = session; if (dropEditReviews) setDraftReviews(null); - setInput((current) => joinDraftText(preEditDraft.text, current)); - if (preEditDraft.attachments.length > 0) { - setAttachments((current) => [...preEditDraft.attachments, ...current]); + // Its composer unmounted (a workspace switch): every note goes to the review store. + if (!isMountedRef.current) { + moveEditToDraft(session); + return true; } + session.settled = true; + const { preEditReviews } = session; + // The composer goes back to the unsent draft; what was typed in the edit buffer while the + // send was in flight joins it after, never replacing it. + appendToDraft(endEditDraft()); if (preEditReviews !== null) { if ((dropEditReviews || draftReviewsRef.current === null) && onAddReviewForRestore) { // The notes in effect live in the review store: add the restored ones there too. @@ -1290,6 +1301,52 @@ const ChatInputInner: React.FC = (props) => { // By identity, not row id: a row reopened after a cancel is a new edit. const isOpenEditOrNone = (session: EditSession | null) => editingMessageIdRef.current === undefined || editSessionRef.current === session; + // An unmounted composer's edit already ended: ChatPane's one edit slot may hold another + // workspace's edit by now, which a cancel from here would close. + const mayCancelEdit = (session: EditSession | null) => + isMountedRef.current && isOpenEditOrNone(session); + /** Text after the unsent draft, files after its files: never over it. */ + const appendToDraft = (contents: { text: string; attachments: ChatAttachment[] } | null) => { + if (contents && contents.text.trim().length > 0) { + getDraftStore().setText(draftScope, (current) => joinDraftText(current, contents.text)); + } + if (contents && contents.attachments.length > 0) { + getDraftStore().setAttachments(draftScope, (current) => [ + ...current, + ...contents.attachments, + ]); + } + }; + // The one move of an edit into the workspace's draft (#5808): settle, then take the buffer, so + // a second call finds nothing. Text and files go after the unsent draft; the pre-edit and edit + // notes go to the review store. Mounted or not: the composer belongs to one workspace. + const moveEditToDraft = (session: EditSession) => { + session.settled = true; + appendToDraft(endEditDraft()); + const notes = [...(session.preEditReviews ?? []), ...(draftReviewsRef.current ?? [])]; + for (const review of notes) onAddReviewForRestore?.(review); + setDraftReviews(null); + }; + // An edit that ended unsettled moves to the draft: its composer unmounted, or ChatPane + // cleared or replaced its target (row deleted, no refresh target, a second Edit). An edit send + // in flight keeps it: an accepted edit replaces its row before the reply. + const releaseEndedEdit = () => { + const session = editSessionRef.current; + if (!session || session.settled || session.sendInFlight) return; + if (isMountedRef.current && editingMessageIdRef.current === session.id) return; + moveEditToDraft(session); + }; + const markEditSendInFlight = (session: EditSession | null, inFlight: boolean) => { + if (session) session.sendInFlight = inFlight; + }; + // After every commit: the edit's end arrives as a prop change, and settled sessions no-op. + // PERF: this runs on every ChatInputInner commit, stream updates included. Keep it O(1) with + // no I/O: releaseEndedEdit reads only refs and returns before any store read when no edit + // session exists (or it is settled, in flight, or still open). + useEffect(() => { + releaseEndedEditRef.current = releaseEndedEdit; + releaseEndedEdit(); + }); // Method to restore text to input (used by compaction cancel) const restoreText = useCallback( @@ -1397,35 +1454,29 @@ const ChatInputInner: React.FC = (props) => { }; }, [focusMessageInput, openModelSelector]); - // When entering editing mode, save current draft and populate with message content. - // Runs once per edit target: the draft callbacks change identity as the user types, and - // re-applying would clobber the in-progress edit text. The applied-id ref makes that - // explicit instead of hiding the callbacks from the dependency list. + // When entering editing mode, fill the edit buffer with the message content; the unsent draft + // stays as it is. Runs once per edit target: the draft callbacks change identity as the user + // types, and re-applying would clobber the in-progress edit text. The applied-id ref makes + // that explicit instead of hiding the callbacks from the dependency list. const appliedEditIdRef = useRef(null); - const draftPayloadsLoaded = draft.payloadsLoaded; useEffect(() => { if (!editingMessage) { appliedEditIdRef.current = null; return; } if (appliedEditIdRef.current === editingMessage.id) return; - if (!draftPayloadsLoaded) { - // Hydrated attachments have no payloads yet (the draft shows none). Snapshotting now would - // save an attachment-less draft on cancel, and the edit's full replacement would end the - // load. Enter edit mode once they load (re-requested here in case an earlier load failed). - getDraftStore() - .ensurePayloads(draftScope) - .catch((error: unknown) => console.warn("Failed to load draft attachments:", error)); - return; - } appliedEditIdRef.current = editingMessage.id; + // A second Edit: the after-commit effect above moved the previous one, so read the ref. editSessionRef.current = { id: editingMessage.id, - preEditDraft: getDraft(), - preEditReviews: draftReviews, + preEditReviews: draftReviewsRef.current, settled: false, + sendInFlight: false, }; - applyDraftFromPending(editingMessage.pending, `edit-${editingMessage.id}`); + beginEditDraft(editingMessage.id, { + text: editingMessage.pending.content, + attachments: pendingChatAttachments(editingMessage.pending, `edit-${editingMessage.id}`), + }); setDraftReviews(editingMessage.pending.reviews); // Auto-resize textarea and focus setTimeout(() => { @@ -1436,15 +1487,7 @@ const ChatInputInner: React.FC = (props) => { inputRef.current.focus(); } }, 0); - }, [ - editingMessage, - draftPayloadsLoaded, - draftScope, - getDraft, - draftReviews, - applyDraftFromPending, - setDraftReviews, - ]); + }, [editingMessage, draftReviewsRef, beginEditDraft, setDraftReviews]); // Project live workflow run cards for foreground slash invocations after reloads. useEffect(() => { @@ -2069,7 +2112,10 @@ const ChatInputInner: React.FC = (props) => { for (const action of actions) { switch (action.type) { case "clear-input": - if (!editCancelled()) setInput(""); + // An editing command clears its edit's buffer only: its row can already be gone. + if (commandEditSession) { + if (!commandEditSession.settled) updateEditDraft(commandEditSession.id, { text: "" }); + } else if (!editCancelled()) setInput(""); break; case "reset-input-height": if (inputRef.current) inputRef.current.style.height = ""; @@ -2087,7 +2133,11 @@ const ChatInputInner: React.FC = (props) => { setSendingCount((count) => count + (action.sending ? 1 : -1)); break; case "clear-attachments": - if (!editCancelled()) setAttachments([]); + if (commandEditSession) { + if (!commandEditSession.settled) { + updateEditDraft(commandEditSession.id, { attachments: [] }); + } + } else if (!editCancelled()) setAttachments([]); break; case "detach-reviews": if (variant === "workspace") props.onDetachAllReviews?.(); @@ -2105,7 +2155,7 @@ const ChatInputInner: React.FC = (props) => { case "cancel-edit": // Emitted once an editing command (/compact) was accepted: the edit is complete. restoredPreEditDraft = restorePreEditDraftAfterSend(commandEditSession, true); - if (isOpenEditOrNone(commandEditSession)) commandOnCancelEdit?.(); + if (mayCancelEdit(commandEditSession)) commandOnCancelEdit?.(); break; case "edit-history-changed": startEditTranscriptRefresh(action.editMessageId, action.precondition); @@ -2137,7 +2187,7 @@ const ChatInputInner: React.FC = (props) => { // Async phases can outlive the invoking render, so check the live // draft: the getDraft closure captured here still reports // this render's input and would refuse to restore over a newer draft. - if (getDraftStore().getText(draftScope).trim().length === 0) { + if (getLiveText().trim().length === 0) { setInput(restoreInput); } else { setDraftReviews(null); @@ -2172,7 +2222,8 @@ const ChatInputInner: React.FC = (props) => { editMessageId: string, precondition: HistoryEditPrecondition ): void => { - if (variant !== "workspace") return; + // An unmounted composer's edit ended with it. + if (variant !== "workspace" || !isMountedRef.current) return; const targetWorkspaceId = props.workspaceId; const onEditingMessageChange = props.onEditingMessageChange; const onCancelEdit = props.onCancelEdit; @@ -2206,8 +2257,8 @@ const ChatInputInner: React.FC = (props) => { return; case "target-not-found": if (!isSameEdit()) return; - // Leave edit mode without restoring the pre-edit draft: the typed text stays in - // the composer as a normal draft. + // Leave edit mode without cancelling: releaseEndedEdit keeps the typed text (and + // its notes) in the composer as a normal draft, after the unsent draft. onCancelEdit?.(); if (isMountedRef.current) { pushToast({ type: "error", message: EDIT_TARGET_GONE_MESSAGE }); @@ -2263,9 +2314,19 @@ const ChatInputInner: React.FC = (props) => { const onEditSendPendingChange = variant === "workspace" && editingMessageForUi ? props.onEditSendPendingChange : undefined; onEditSendPendingChange?.(true); + const editSession = + editingMessageForUi && editSessionRef.current?.id === editingMessageForUi.id + ? editSessionRef.current + : null; + markEditSendInFlight(editSession, true); await runWithFinally( () => sendComposerInput(overrides), - () => onEditSendPendingChange?.(false) + () => { + onEditSendPendingChange?.(false); + markEditSendInFlight(editSession, false); + // The edit can end while its send runs (a refusal whose refresh finds no target). + releaseEndedEdit(); + } ); }; const sendComposerInput = async (overrides?: InternalSendOverrides) => { @@ -2670,12 +2731,11 @@ const ChatInputInner: React.FC = (props) => { // after it (another window that still held it saved it, #5501). A match inside a longer // line is the user's own text, so the sent text comes back beside it: a duplicate // beats a loss. - setInput((current) => - hasDraftBlock(current, text) ? current : joinDraftText(text, current) - ); + const withSentText = (current: string) => + hasDraftBlock(current, text) ? current : joinDraftText(text, current); // On an id match the sent copy wins: it may be the staged version of a file that // another window saved again as pending (#5501), and a retry must not stage it twice. - setAttachments((current) => { + const withSentAttachments = (current: ChatAttachment[]) => { const sentById = new Map( sendAttachments.map((attachment) => [attachment.id, attachment]) ); @@ -2684,7 +2744,36 @@ const ChatInputInner: React.FC = (props) => { const merged = current.map((attachment) => sentById.get(attachment.id) ?? attachment); const changed = merged.some((attachment, index) => attachment !== current[index]); return missing.length > 0 || changed ? [...missing, ...merged] : current; - }); + }; + if (!editSessionForSend) { + setInput(withSentText); + setAttachments(withSentAttachments); + return; + } + // A refused edit goes back into its own buffer while unsettled, shown or not. Else after + // the unsent draft: setInput could reach a newer edit's buffer (#5808). + const backInEdit = + !editSessionForSend.settled && + updateEditDraft(editSessionForSend.id, (edit) => ({ + text: withSentText(edit.text), + attachments: withSentAttachments(edit.attachments), + })); + if (!backInEdit) appendToDraft({ text, attachments: sendAttachments }); + }; + const editSendIsLive = () => + editSessionForSend?.settled === false && editSessionRef.current === editSessionForSend; + // A refused edit's notes follow its session: in front of its notes (never twice), or to + // the review store once it is gone, never into another edit's or the draft's override. + const putBackSendReviews = () => { + if (!editSessionForSend) setDraftReviews(preSendReviews); + else if (!editSendIsLive()) { + for (const review of preSendReviews ?? []) onAddReviewForRestore?.(review); + } else if (preSendReviews !== null) { + setDraftReviews((current) => [ + ...preSendReviews.filter((review) => !current?.includes(review)), + ...(current ?? []), + ]); + } }; const preSendReviews = draftReviews; const editMessageForSend = editingMessageForUi; @@ -2837,6 +2926,8 @@ const ChatInputInner: React.FC = (props) => { pushToast({ type: "error", message: TRANSCRIPT_NOT_CAUGHT_UP_MESSAGE }); return; } + // An edit cancelled (or moved to the draft) while this send prepared sends nothing. + if (editMessageForSend && !editSendIsLive()) return; // Idempotent sends (formal/composer-drafts/ComposerSends.tla, FixRenderer): every send // carries an id minted here; a retry reuses it with the same request, so the backend @@ -2954,7 +3045,7 @@ const ChatInputInner: React.FC = (props) => { // through its draft entry instead) setOptimisticallyDismissedEditId(null); putBackTaken(); - setDraftReviews(preSendReviews); + putBackSendReviews(); // The rows this edit would delete changed after its evidence was captured. Stay in // edit mode with the draft, block Send, and re-read the transcript so the user can // review it and send again explicitly (never automatically). @@ -3014,7 +3105,7 @@ const ChatInputInner: React.FC = (props) => { // Exit editing mode if we were editing restorePreEditDraftAfterSend(editSessionForSend); - if (editMessageForSend && props.onCancelEdit && isOpenEditOrNone(editSessionForSend)) { + if (editMessageForSend && props.onCancelEdit && mayCancelEdit(editSessionForSend)) { props.onCancelEdit(); } else if (editMessageForSend) { setOptimisticallyDismissedEditId(null); @@ -3034,7 +3125,7 @@ const ChatInputInner: React.FC = (props) => { // Restore draft on error setOptimisticallyDismissedEditId(null); putBackTaken(); - setDraftReviews(preSendReviews); + putBackSendReviews(); }; await runWithCatchFinally(sendPreparedMessage, restoreDraftOnError, () => { setSendingCount((c) => c - 1); diff --git a/src/browser/features/ChatInput/useComposerDraft.ts b/src/browser/features/ChatInput/useComposerDraft.ts index 3a60186b16..970a844b7c 100644 --- a/src/browser/features/ChatInput/useComposerDraft.ts +++ b/src/browser/features/ChatInput/useComposerDraft.ts @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from "react"; +import { useEffect, useLayoutEffect, useRef, useState } from "react"; import { defaultCreationDraftScope, getDraftStore, @@ -16,6 +16,8 @@ interface UseComposerDraftOptions { workspaceId: string | null; creationProjectPath: string; pendingDraftId?: string; + /** The message being edited, if any: its text lives in the edit buffer below. */ + editMessageId?: string; attachedReviews: Review[]; pushToast: (toast: Omit & { type: Toast["type"] | "info" }) => void; } @@ -43,6 +45,18 @@ export function getComposerDraftScope(options: { : defaultCreationDraftScope(options.creationProjectPath); } +/** The open edit's text and attachments. Memory only: see useComposerDraft. */ +interface EditDraft { + editId: string; + text: string; + attachments: ChatAttachment[]; +} +type EditPatch = Partial>; + +type Update = T | ((previous: T) => T); +const applyUpdate = (value: Update, previous: T): T => + typeof value === "function" ? (value as (previous: T) => T)(previous) : value; + export function useComposerDraft(options: UseComposerDraftOptions) { const { attachedReviews, pushToast } = options; const draftStore = getDraftStore(); @@ -50,17 +64,43 @@ export function useComposerDraft(options: UseComposerDraftOptions) { // Drafts live in the in-memory DraftStore, persisted to the backend in the background. The // rendered text never waits for (or depends on) a storage write succeeding (issue 5006). const draft = useDraft(draftScope); - const input = draft.text; - const attachments = draft.attachments; - const setInput = (value: string | ((previous: string) => string)) => - draftStore.setText(draftScope, value); + // While a message is edited, the composer edits this buffer instead of the draft: the edit + // text stays in this window's memory, so a reload keeps the unsent draft (#5672) and another + // window never shows the edit (#5571). A reload drops the edit; that is the chosen tradeoff. + // The ref is the live copy for writes that run after an await; renders read the state. + const [editDraft, setEditDraftState] = useState(null); + const editDraftRef = useRef(null); + const editIdRef = useRef(options.editMessageId); + useLayoutEffect(() => { + editIdRef.current = options.editMessageId; + }); + const writeEditDraft = (next: EditDraft | null) => { + editDraftRef.current = next; + setEditDraftState(next); + }; + // Only the open edit's buffer counts. One left behind by an edit that ended without settling + // (its row was replaced) is ignored until the composer moves it to the draft. + const liveEditDraft = () => { + const current = editDraftRef.current; + return current !== null && current.editId === editIdRef.current ? current : null; + }; + const editActive = editDraft !== null && editDraft.editId === options.editMessageId; + const input = editActive ? editDraft.text : draft.text; + const attachments = editActive ? editDraft.attachments : draft.attachments; + const setInput = (value: Update) => { + const edit = liveEditDraft(); + if (edit) writeEditDraft({ ...edit, text: applyUpdate(value, edit.text) }); + else draftStore.setText(draftScope, value); + }; const latestInputValueRef = useRef(input); latestInputValueRef.current = input; // Synchronous: the store applies the change before returning, so a Stop restore can flush it // right after this call (#4448) even if the composer unmounts before the next render. - const setAttachments = ( - value: ChatAttachment[] | ((previous: ChatAttachment[]) => ChatAttachment[]) - ) => draftStore.setAttachments(draftScope, value); + const setAttachments = (value: Update) => { + const edit = liveEditDraft(); + if (edit) writeEditDraft({ ...edit, attachments: applyUpdate(value, edit.attachments) }); + else draftStore.setAttachments(draftScope, value); + }; const pushToastRef = useRef(pushToast); pushToastRef.current = pushToast; const { variant, workspaceId, creationProjectPath, pendingDraftId } = options; @@ -93,7 +133,14 @@ export function useComposerDraft(options: UseComposerDraftOptions) { .catch(() => undefined); }; }, [variant, workspaceId, creationProjectPath, pendingDraftId]); - const [draftReviews, setDraftReviews] = useState(null); + const [draftReviews, setDraftReviewsState] = useState(null); + // Written with the state, so an edit send that completes after its composer unmounted still + // reads (and moves) the notes it put back (#5808). Never written on an edit keystroke. + const draftReviewsRef = useRef(draftReviews); + const setDraftReviews = (value: Update) => { + draftReviewsRef.current = applyUpdate(value, draftReviewsRef.current); + setDraftReviewsState(draftReviewsRef.current); + }; const draftReviewIdsRef = useRef(new WeakMap()); const nextDraftReviewIdRef = useRef(0); const isDraftReviewData = (value: unknown): value is ReviewNoteDataForDisplay => @@ -144,12 +191,32 @@ export function useComposerDraft(options: UseComposerDraftOptions) { draftScope, input, setInput, - payloadsLoaded: draft.payloadsLoaded, + /** The live composer text (the open edit's, else the draft's), for code after an await. */ + getLiveText: () => liveEditDraft()?.text ?? draftStore.getText(draftScope), + /** Fill the edit buffer; from now on the composer edits it, not the draft. */ + beginEditDraft: (editId: string, next: { text: string; attachments: ChatAttachment[] }) => + writeEditDraft({ editId, ...next }), + /** Change this edit's buffer, shown or not; never the draft. False if it is not this edit's. */ + updateEditDraft: (editId: string, update: EditPatch | ((edit: EditDraft) => EditPatch)) => { + const edit = editDraftRef.current; + if (edit?.editId !== editId) return false; + writeEditDraft({ ...edit, ...(typeof update === "function" ? update(edit) : update) }); + return true; + }, + /** Drop the edit buffer and return what it held (text typed during an edit send). */ + endEditDraft: () => { + const edit = editDraftRef.current; + writeEditDraft(null); + return edit; + }, + // An edit's attachments come from its message, complete. + payloadsLoaded: editActive || draft.payloadsLoaded, unresolvedSendCount: draft.unresolvedSendCount, latestInputValueRef, attachments, setAttachments, draftReviews, + draftReviewsRef, setDraftReviews, getDraft, setDraft, diff --git a/tests/bugbash/repros/knownFailureReloadDuringEdit.e2e.ts b/tests/bugbash/repros/reloadDuringEditKeepsDraft.e2e.ts similarity index 84% rename from tests/bugbash/repros/knownFailureReloadDuringEdit.e2e.ts rename to tests/bugbash/repros/reloadDuringEditKeepsDraft.e2e.ts index 11122f84ba..fd1a406f75 100644 --- a/tests/bugbash/repros/knownFailureReloadDuringEdit.e2e.ts +++ b/tests/bugbash/repros/reloadDuringEditKeepsDraft.e2e.ts @@ -1,5 +1,5 @@ -// 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. +// #5672: a reload during a message edit must keep the unsent draft. The edit text lives in the +// window's memory only, so the reload drops the edit and the composer shows the unsent draft. import { test } from "@e2e-dev/web"; import { expect } from "e2e"; import { openPlayground, sendMessageForEdit, WORKSPACE_TITLE } from "./helpers"; @@ -9,7 +9,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(); diff --git a/tests/ui/chat/composerDraftsFormalRepro.test.ts b/tests/ui/chat/composerDraftsFormalRepro.test.ts index 83c2dfd4d7..6b7b9ccc5b 100644 --- a/tests/ui/chat/composerDraftsFormalRepro.test.ts +++ b/tests/ui/chat/composerDraftsFormalRepro.test.ts @@ -26,7 +26,8 @@ import { fireEvent, waitFor } from "@testing-library/react"; import * as fs from "fs/promises"; import * as path from "path"; -import { getDraftStore } from "@/browser/stores/DraftStore"; +import { DraftStore, getDraftStore } from "@/browser/stores/DraftStore"; +import { createTestApiClient } from "@/browser/testUtils"; import type { DraftScope } from "@/common/orpc/schemas/drafts"; import { preloadTestModules } from "../../ipc/setup"; import { createAppHarness, type AppHarness } from "../harness"; @@ -201,15 +202,16 @@ describe("formal/composer-drafts: composer text across a failed send", () => { /** * #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. + * and puts what it took back when it fails. Since #5571 the edit text lives in this window's + * memory, so no other window can hold it or save it again: the put-back goes into the edit, and + * what another window saves meanwhile stays in the shared draft. */ describe("formal/composer-drafts: a failed edit's put-back", () => { beforeAll(async () => { await preloadTestModules(); }); - async function startEdit(app: AppHarness, scope: DraftScope) { + async function startEdit(app: AppHarness) { await app.chat.send("first message"); await app.chat.expectTranscriptContains("Mock response: first message", WAIT.timeout); await app.chat.expectStreamComplete(); @@ -227,7 +229,7 @@ describe("formal/composer-drafts: a failed edit's put-back", () => { expect(element.value).toBe("first message"); return element; }, WAIT); - getDraftStore().setText(scope, "edited message"); + fireEvent.change(textarea, { target: { value: "edited message" } }); await waitFor(() => expect(textarea.value).toBe("edited message"), WAIT); return textarea; } @@ -239,10 +241,10 @@ describe("formal/composer-drafts: a failed edit's put-back", () => { ); } - test("does not show the edit's text twice when another window saved it meanwhile", async () => { + test("puts the edit's text back into the edit once, apart from another window's draft", 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) => { + const textarea = await startEdit(app).catch(async (error: unknown) => { await app.dispose(); throw error; }); @@ -250,18 +252,23 @@ describe("formal/composer-drafts: a failed edit's put-back", () => { 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); + await waitFor(() => expect(textarea.value).toBe(""), WAIT); - // 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"; + // A second window saves its own draft while the edit send is in flight. + const saved = "typed 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. + // Target assertion: the edit gets its text back once, and the shared draft holds only the + // other window's text. + await waitFor(() => { + const edit = app.view.container.querySelector( + 'textarea[aria-label="Edit message"]' + ); + expect(edit?.value).toBe("edited message"); + }, WAIT); expect(getDraftStore().getView(scope).text).toBe(saved); await getDraftStore().flush(scope); expect((await app.env.services.draftService.get(scope)).text).toBe(saved); @@ -272,68 +279,89 @@ describe("formal/composer-drafts: a failed edit's put-back", () => { } }, 120_000); - test("keeps the staged copy of an attachment another window saved again as pending", async () => { + // Replaces the pre-#5571 "another window saved the staged copy again" case: no other window + // holds an edit now, but a failed edit send must still neither lose nor duplicate a staged file. + test("keeps a staged attachment in the edit once across a refused send, out of the shared draft", 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()); + const secondWindow = new DraftStore(); + secondWindow.setClient(createTestApiClient(app.env.orpc)); + const stageSpy = jest.spyOn(app.env.services.workspaceService, "stageAttachment"); + const chips = () => + ( + app.view.container.querySelector('[data-component="ChatInputSection"]')?.textContent ?? "" + ).split("notes.md").length - 1; + const unsent = { + kind: "provider" as const, + id: "file-unsent", + url: "data:text/plain;base64,dW5zZW50", + mediaType: "text/plain", + filename: "unsent.txt", + }; + let held: ReturnType | null = null; 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 + await secondWindow.whenReady(); + // The message to edit carries a file staged into the workspace. + const input = await waitFor(() => { + const element = app.view.container.querySelector( + '[data-component="ChatInputSection"] input[type="file"]' + ); + if (!element) throw new Error("File input not found"); + return element; + }, WAIT); + fireEvent.change(input, { + target: { files: [new File(["# notes"], "notes.md", { type: "text/markdown" })] }, + }); + await waitFor(() => expect(chips()).toBe(1), WAIT); + await app.chat.send("first message"); + await app.chat.expectTranscriptContains("Mock response: first message", WAIT.timeout); + await app.chat.expectStreamComplete(); + expect(stageSpy).toHaveBeenCalledTimes(1); + // An unsent draft with its own file, then the edit: the edit shows the message's file. + getDraftStore().setText(scope, "unsent draft"); + getDraftStore().setAttachments(scope, [unsent]); + await waitFor(() => expect(secondWindow.getText(scope)).toBe("unsent draft"), WAIT); + fireEvent.click( + await waitFor(() => { + const button = app.view.container.querySelector('button[aria-label="Edit"]'); + if (!button) throw new Error("Edit button not found"); + return button; + }, WAIT) ); + const textarea = await waitFor(() => { + const element = app.view.container.querySelector( + 'textarea[aria-label="Edit message"]' + ); + if (!element) throw new Error("Edit textarea not found"); + return element; + }, WAIT); + fireEvent.change(textarea, { target: { value: "edited message" } }); + await waitFor(() => expect(chips()).toBe(1), WAIT); + held = holdSendReplies(app, () => refused()); + fireEvent.keyDown(textarea, { key: "Enter" }); + await waitFor(() => expect(held?.spy).toHaveBeenCalledTimes(1), 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", - ]); + // Target assertion: the refused edit keeps its text and the staged file, once, without + // staging it again; the shared draft and the other window keep only the unsent draft. + await waitFor(() => expect(textarea.value).toBe("edited message"), WAIT); + expect(chips()).toBe(1); + expect(stageSpy).toHaveBeenCalledTimes(1); + const sharedIds = (view: { attachments: { id: string }[] }) => + view.attachments.map(({ id }) => id); + expect(sharedIds(getDraftStore().getView(scope))).toEqual(["file-unsent"]); 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", - ]); + expect(sharedIds(await app.env.services.draftService.get(scope))).toEqual(["file-unsent"]); + // Another window loads attachment payloads only for a draft it shows. + await secondWindow.ensurePayloads(scope); + expect(sharedIds(secondWindow.getView(scope))).toEqual(["file-unsent"]); + expect(secondWindow.getText(scope)).toBe("unsent draft"); } finally { - await held.settle(); - held.spy.mockRestore(); + await held?.settle(); + held?.spy.mockRestore(); + stageSpy.mockRestore(); + secondWindow.setClient(null); await app.dispose(); } }, 120_000); diff --git a/tests/ui/chat/editKeepsUnsentDraft.test.ts b/tests/ui/chat/editKeepsUnsentDraft.test.ts index ccbb9241ba..d285b39197 100644 --- a/tests/ui/chat/editKeepsUnsentDraft.test.ts +++ b/tests/ui/chat/editKeepsUnsentDraft.test.ts @@ -11,22 +11,47 @@ jest.mock("lottie-react", () => ({ import { act, fireEvent, waitFor, within } from "@testing-library/react"; import { updatePersistedState } from "@/browser/hooks/usePersistedState"; -import { getDraftStore } from "@/browser/stores/DraftStore"; +import { DraftStore, getDraftStore } from "@/browser/stores/DraftStore"; +import { getReviewStateStore } from "@/browser/stores/ReviewStateStore"; +import { + WorkspaceStore, + useWorkspaceStoreRaw, + workspaceStore, + type WorkspaceState, +} from "@/browser/stores/WorkspaceStore"; +import { createTestApiClient } from "@/browser/testUtils"; import { getAutoCompactionThresholdKey } from "@/common/constants/storage"; import type { DraftScope } from "@/common/orpc/schemas/drafts"; import type { ReviewNoteData } from "@/common/types/review"; import { EDIT_HISTORY_CHANGED_MESSAGE } from "@/constants/transcriptBarrier"; import { WORKSPACE_DEFAULTS } from "@/constants/workspaceDefaults"; import { Err } from "@/common/types/result"; +import { joinDraftText } from "@/common/utils/composerDraftText"; +import { detectDefaultTrunkBranch } from "@/node/git"; +import { generateBranchName } from "../../ipc/helpers"; import { preloadTestModules } from "../../ipc/setup"; -import { createAppHarness, type AppHarness } from "../harness"; +import { ChatHarness, createAppHarness, type AppHarness } from "../harness"; const LOAD_TOLERANT_WAIT = { timeout: 30_000 }; -async function startEditWithUnsentDraft(app: AppHarness, scope: DraftScope) { +/** + * The mock's reply to `text`. It echoes the sent text, with its one note formatted in front, + * so a reply to a message with a note is matched within that one reply. + */ +const mockReply = (text: string, withNote: boolean) => + withNote + ? new RegExp(`Mock response: [^<]*\\s*${text}`) + : `Mock response: ${text}`; + +/** `rowNote`: a review note sent with the edited row, so the edit opens with it. */ +async function startEditWithUnsentDraft(app: AppHarness, scope: DraftScope, rowNote?: string) { + if (rowNote) { + await attachStoreReview(app, "review-row", rowNote); + await waitFor(() => expect(composerText(app)).toContain(rowNote), LOAD_TOLERANT_WAIT); + } await app.chat.send("first message"); await app.chat.expectTranscriptContains( - "Mock response: first message", + mockReply("first message", rowNote !== undefined), LOAD_TOLERANT_WAIT.timeout ); await app.chat.expectStreamComplete(); @@ -72,18 +97,210 @@ async function expectUnsentDraftKept(app: AppHarness, scope: DraftScope) { expect(saved.attachments.map(({ id }) => id)).toEqual(["file-unsent"]); } +/** The composer's normal (not edit) textarea. */ +function messageTextarea(app: AppHarness): HTMLTextAreaElement { + const textarea = app.view.container.querySelector( + 'textarea[aria-label="Message"]' + ); + if (!textarea) throw new Error("Message textarea not found"); + return textarea; +} + +/** An edit that ended unsettled: its text follows the unsent draft, which keeps its file. */ +async function expectEditKeptAsDraft(app: AppHarness, scope: DraftScope) { + await waitFor(() => { + const value = messageTextarea(app).value; + expect(value.startsWith("unsent draft")).toBe(true); + expect(value.trimEnd().endsWith("edited message")).toBe(true); + }, LOAD_TOLERANT_WAIT); + expect(getDraftStore().getText(scope)).toBe(messageTextarea(app).value); + expect( + getDraftStore() + .getView(scope) + .attachments.map(({ id }) => id) + ).toEqual(["file-unsent"]); +} + +/** Show a workspace the way the sidebar does, and wait until its composer is mounted. */ +async function showWorkspace(app: AppHarness, workspaceId: string, name: string) { + const row = await waitFor(() => { + const element = app.view.container.querySelector(`[data-workspace-id="${workspaceId}"]`); + if (!element || element.getAttribute("aria-disabled") === "true") { + throw new Error("Workspace row not selectable yet"); + } + return element as HTMLElement; + }, LOAD_TOLERANT_WAIT); + fireEvent.click(row); + workspaceStore.setActiveWorkspaceId(workspaceId); + await waitFor(() => { + expect(document.title.startsWith(name)).toBe(true); + expect(app.view.container.querySelector('[data-testid="message-window"]')).not.toBe(null); + }, LOAD_TOLERANT_WAIT); +} + +/** Stage a file in the composer, so the next sent message (and an edit of it) carries it. */ +async function attachComposerFile(app: AppHarness, filename: string) { + const input = await waitFor(() => { + const element = app.view.container.querySelector( + '[data-component="ChatInputSection"] input[type="file"]' + ); + if (!element) throw new Error("File input not found"); + return element; + }, LOAD_TOLERANT_WAIT); + fireEvent.change(input, { + target: { files: [new File(["# file"], filename, { type: "text/markdown" })] }, + }); + await waitFor(() => expect(composerText(app)).toContain(filename), LOAD_TOLERANT_WAIT); +} + +/** + * An edit that lost its target without a settle: its text follows the unsent draft, and its + * file joins the unsent draft's file, in memory and on the backend. + */ +async function expectEditContentsInDraft(app: AppHarness, scope: DraftScope, filename: string) { + await waitFor(() => { + const text = getDraftStore().getText(scope); + expect(text.startsWith("unsent draft")).toBe(true); + expect(text.trimEnd().endsWith("edited message")).toBe(true); + }, LOAD_TOLERANT_WAIT); + const names = (attachments: { filename?: string; id: string }[]) => + attachments.map((attachment) => attachment.filename ?? attachment.id); + expect(names(getDraftStore().getView(scope).attachments)).toEqual(["unsent.txt", filename]); + await getDraftStore().flush(scope); + const saved = await app.env.services.draftService.get(scope); + expect(saved.text).toBe(getDraftStore().getText(scope)); + expect(saved.attachments).toHaveLength(2); +} + +/** Another renderer on the same backend: a reload of this window, or a second window. */ +async function otherRenderer(app: AppHarness): Promise { + const store = new DraftStore(); + store.setClient(createTestApiClient(app.env.orpc)); + await store.whenReady(); + return store; +} + +/** + * Type into the open edit textarea. The edit text lives in the composer's memory, not in the + * draft store, so it is set through the textarea as a user would. + */ +function typeIntoEdit(textarea: HTMLTextAreaElement, text: string) { + fireEvent.change(textarea, { target: { value: text } }); +} + describe("Completing an edit of an older message", () => { beforeAll(async () => { await preloadTestModules(); }); + // The edit text lives in this window's memory only: the shared draft keeps the unsent draft, + // so a reload (#5672) and a second window (#5571) both see the unsent draft, never the edit. + test("a reload during an edit keeps the unsent draft (#5672)", async () => { + const app = await createAppHarness({ branchPrefix: "edit-reload-keeps-draft" }); + let reloaded: DraftStore | null = null; + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + const textarea = await startEditWithUnsentDraft(app, scope); + // Typing in the edit writes only the memory buffer, never the persisted draft store. + const setText = jest.spyOn(getDraftStore(), "setText"); + const setAttachments = jest.spyOn(getDraftStore(), "setAttachments"); + // Several keystrokes, as a user types: none of them reaches the draft store. + for (const typed of ["e", "ed", "edi", "edit", "edited before reload"]) { + typeIntoEdit(textarea, typed); + await waitFor(() => expect(textarea.value).toBe(typed)); + } + expect(setText).not.toHaveBeenCalled(); + expect(setAttachments).not.toHaveBeenCalled(); + setText.mockRestore(); + setAttachments.mockRestore(); + await getDraftStore().flush(scope); + expect((await app.env.services.draftService.get(scope)).text).toBe("unsent draft"); + reloaded = await otherRenderer(app); + expect(reloaded.getText(scope)).toBe("unsent draft"); + const saved = await app.env.services.draftService.get(scope); + expect(saved.attachments.map(({ id }) => id)).toEqual(["file-unsent"]); + } finally { + reloaded?.setClient(null); + await app.dispose(); + } + }, 120_000); + + // An unsettled edit that loses its target keeps its contents in the workspace's draft, after + // the unsent draft (#5801 review): here the workspace turns transcript-only mid-edit, and the + // composer is replaced by the read-only notice. + test("an edit keeps its text and files in the draft when the workspace turns transcript-only", async () => { + const app = await createAppHarness({ branchPrefix: "edit-transcript-only-keeps" }); + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + await attachComposerFile(app, "edit-file.md"); + const textarea = await startEditWithUnsentDraft(app, scope); + typeIntoEdit(textarea, "edited message"); + await waitFor(() => expect(textarea.value).toBe("edited message")); + app.env.services.workspaceService.emit("metadata", { + workspaceId: app.workspaceId, + metadata: { ...app.metadata, transcriptOnly: true }, + }); + await waitFor(() => expect(editTextarea(app)).toBeNull(), LOAD_TOLERANT_WAIT); + await expectEditContentsInDraft(app, scope, "edit-file.md"); + } finally { + await app.dispose(); + } + }, 120_000); + + // Same rule when a second Edit replaces the open edit's target: the first edit's contents + // join the draft, and the second edit starts from its own message. + test("a second Edit keeps the first edit's text and files in the draft", async () => { + const app = await createAppHarness({ branchPrefix: "edit-second-edit-keeps" }); + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + await app.chat.send("earlier message"); + await app.chat.expectTranscriptContains( + "Mock response: earlier message", + LOAD_TOLERANT_WAIT.timeout + ); + await app.chat.expectStreamComplete(); + await attachComposerFile(app, "edit-file.md"); + const textarea = await startEditWithUnsentDraft(app, scope); + typeIntoEdit(textarea, "edited message"); + await waitFor(() => expect(textarea.value).toBe("edited message")); + + await editRow(app, "earlier message"); + expect(composerText(app)).not.toContain("edit-file.md"); + await expectEditContentsInDraft(app, scope, "edit-file.md"); + // Cancelling the second edit shows the draft with the first edit's contents. + fireEvent.keyDown(editTextarea(app)!, { key: "Escape" }); + await waitFor(() => expect(editTextarea(app)).toBeNull(), LOAD_TOLERANT_WAIT); + expect(messageTextarea(app).value).toBe(getDraftStore().getText(scope)); + expect(composerText(app)).toContain("edit-file.md"); + } finally { + await app.dispose(); + } + }, 120_000); + + test("an edit in one window does not reach another window's composer (#5571)", async () => { + const app = await createAppHarness({ branchPrefix: "edit-other-window" }); + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + const secondWindow = await otherRenderer(app); + try { + await startEditWithUnsentDraft(app, scope); + await getDraftStore().flush(scope); + // The second window saw the unsent draft arrive; the edit text never follows it. + await waitFor(() => expect(secondWindow.getText(scope)).toBe("unsent draft")); + await getDraftStore().flush(scope); + expect(secondWindow.getText(scope)).toBe("unsent draft"); + } finally { + secondWindow.setClient(null); + await app.dispose(); + } + }, 120_000); + test("keeps the unsent draft, with its attachments", async () => { const app = await createAppHarness({ branchPrefix: "edit-keeps-draft" }); try { const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; const editTextarea = await startEditWithUnsentDraft(app, scope); - getDraftStore().setText(scope, "edited message"); + typeIntoEdit(editTextarea, "edited message"); await waitFor(() => expect(editTextarea.value).toBe("edited message")); fireEvent.keyDown(editTextarea, { key: "Enter" }); @@ -117,7 +334,7 @@ describe("Completing an edit of an older message", () => { return result; }); - getDraftStore().setText(scope, "edited message"); + typeIntoEdit(editTextarea, "edited message"); await waitFor(() => expect(editTextarea.value).toBe("edited message")); fireEvent.keyDown(editTextarea, { key: "Enter" }); await app.chat.expectTranscriptContains("edited message", LOAD_TOLERANT_WAIT.timeout); @@ -131,13 +348,62 @@ describe("Completing an edit of an older message", () => { } }, 120_000); + // The edit can end without the composer settling it: ChatPane drops the edit when its row + // leaves the transcript, or when a history-changed refresh finds no target. The edit's text + // and attachments then stay as a normal draft after the unsent draft, and typing goes there. + test("an edit whose row is deleted stays as a normal draft, after the unsent draft", async () => { + const app = await createAppHarness({ branchPrefix: "edit-row-deleted-typing" }); + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + const textarea = await startEditWithUnsentDraft(app, scope); + typeIntoEdit(textarea, "edited message"); + await waitFor(() => expect(textarea.value).toBe("edited message")); + const cleared = await app.env.services.workspaceService.truncateHistory(app.workspaceId); + expect(cleared.success).toBe(true); + await waitFor(() => expect(editTextarea(app)).toBeNull(), LOAD_TOLERANT_WAIT); + await expectEditKeptAsDraft(app, scope); + + const composer = messageTextarea(app); + // Through the textarea, as a user types: the store shortcut would bypass the composer. + fireEvent.change(composer, { target: { value: "typed after the edit ended" } }); + await app.chat.expectInputValue("typed after the edit ended", LOAD_TOLERANT_WAIT.timeout); + expect(getDraftStore().getText(scope)).toBe("typed after the edit ended"); + } finally { + await app.dispose(); + } + }, 120_000); + + test("an edit whose target a history-changed refresh cannot find stays as a normal draft", async () => { + const app = await createAppHarness({ branchPrefix: "edit-target-gone-keeps-text" }); + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + const textarea = await startEditWithUnsentDraft(app, scope); + const sendSpy = jest + .spyOn(app.env.services.workspaceService, "sendMessage") + .mockResolvedValueOnce(Err({ type: "history-changed" })); + const refreshSpy = jest + .spyOn(WorkspaceStore.prototype, "requestTranscriptRefresh") + .mockResolvedValue({ kind: "target-not-found" }); + typeIntoEdit(textarea, "edited message"); + await waitFor(() => expect(textarea.value).toBe("edited message")); + fireEvent.keyDown(textarea, { key: "Enter" }); + await waitFor(() => expect(refreshSpy).toHaveBeenCalled(), LOAD_TOLERANT_WAIT); + await waitFor(() => expect(editTextarea(app)).toBeNull(), LOAD_TOLERANT_WAIT); + await expectEditKeptAsDraft(app, scope); + refreshSpy.mockRestore(); + sendSpy.mockRestore(); + } finally { + await app.dispose(); + } + }, 120_000); + test("keeps the unsent draft when the edit is a /compact command", async () => { const app = await createAppHarness({ branchPrefix: "edit-compact-keeps-draft" }); try { const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; const editTextarea = await startEditWithUnsentDraft(app, scope); - getDraftStore().setText(scope, "/compact -t 500"); + typeIntoEdit(editTextarea, "/compact -t 500"); await waitFor(() => expect(editTextarea.value).toBe("/compact -t 500")); fireEvent.keyDown(editTextarea, { key: "Enter" }); @@ -200,7 +466,7 @@ describe("Completing an edit of an older message", () => { return realSend(...args); }); - getDraftStore().setText(scope, "/compact -t 500"); + typeIntoEdit(editTextarea, "/compact -t 500"); await waitFor(() => expect(editTextarea.value).toBe("/compact -t 500")); fireEvent.keyDown(editTextarea, { key: "Enter" }); await waitFor(() => expect(sendSpy).toHaveBeenCalled(), LOAD_TOLERANT_WAIT); @@ -260,9 +526,9 @@ const review = (note: string): ReviewNoteData => ({ }); /** Send an edit of the open edit textarea with `text` and wait until its row replaced the old one. */ -async function sendEdit(app: AppHarness, scope: DraftScope, text: string, replaced: string) { +async function sendEdit(app: AppHarness, text: string, replaced: string) { const textarea = editTextarea(app)!; - getDraftStore().setText(scope, text); + typeIntoEdit(textarea, text); await waitFor(() => expect(textarea.value).toBe(text)); fireEvent.keyDown(textarea, { key: "Enter" }); await app.chat.expectTranscriptContains(text, LOAD_TOLERANT_WAIT.timeout); @@ -399,23 +665,27 @@ describe("Edit sends racing newer composer input (#5226)", () => { await preloadTestModules(); }); - test("no new edit starts while an edit send is pending; the unsent draft comes back in order", async () => { + test("no new edit starts while an edit send is pending; the unsent draft is back once it is accepted", async () => { const app = await createAppHarness({ branchPrefix: "edit-refused-while-pending" }); try { const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; await startEditWithUnsentDraft(app, scope); // Hold the edit's reply, and its stream at start: the composer is usable meanwhile. const replies = holdSendReplies(app); - await sendEdit(app, scope, "[mock:wait-start] edited message", "first message"); + await sendEdit(app, "[mock:wait-start] edited message", "first message"); - // A second edit (the row's Edit action) and a third (ArrowUp in the empty composer). + // The edit text never replaced the unsent draft: once the edited row is replaced, the + // composer shows the draft again, before the edit's reply. + await app.chat.expectInputValue("unsent draft", LOAD_TOLERANT_WAIT.timeout); + // A second edit (the row's Edit action) and a third (ArrowUp in an emptied composer). await expectEditRefused(app, "edited message"); + await app.chat.typeWithoutSending(""); await act(async () => { fireEvent.keyDown(composerHolding(app, ""), { key: "ArrowUp" }); await new Promise((resolve) => setTimeout(resolve, 50)); }); expect(editTextarea(app)).toBeNull(); - await app.chat.typeWithoutSending("typed meanwhile"); + await app.chat.typeWithoutSending("unsent draft\n\ntyped meanwhile"); replies.release(); app.env.services.aiService.releaseMockStreamStartGate(app.workspaceId); @@ -458,7 +728,7 @@ describe("Edit sends racing newer composer input (#5226)", () => { await sendGate; return realSend(...args); }); - getDraftStore().setText(scope, "/compact -t 500"); + typeIntoEdit(editTextarea0, "/compact -t 500"); await waitFor(() => expect(editTextarea0.value).toBe("/compact -t 500")); fireEvent.keyDown(editTextarea0, { key: "Enter" }); await waitFor(() => expect(sendSpy).toHaveBeenCalled(), LOAD_TOLERANT_WAIT); @@ -480,7 +750,7 @@ describe("Edit sends racing newer composer input (#5226)", () => { const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; const textarea = await startEditWithUnsentDraft(app, scope); const save = await holdNextSendBeforeClear(app); - getDraftStore().setText(scope, "edited message"); + typeIntoEdit(textarea, "edited message"); await waitFor(() => expect(textarea.value).toBe("edited message")); fireEvent.keyDown(textarea, { key: "Enter" }); @@ -512,7 +782,7 @@ describe("Edit sends racing newer composer input (#5226)", () => { await editRow(app, "first message"); const replies = holdSendReplies(app); - await sendEdit(app, scope, "edited message", "first message"); + await sendEdit(app, "edited message", "first message"); await app.chat.expectStreamComplete(); // A note attached while the edit's reply is pending. await attachStoreReview(app, "review-late", "late note"); @@ -531,25 +801,29 @@ describe("Edit sends racing newer composer input (#5226)", () => { } }, 120_000); - test("a follow-up sent while an edit is pending clears only its own text", async () => { + test("a follow-up sent while an edit is pending gets nothing merged in when the edit completes", async () => { const app = await createAppHarness({ branchPrefix: "edit-followup-keeps-draft" }); try { const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; await startEditWithUnsentDraft(app, scope); // Hold the edit's reply, and its stream at start: the composer is usable meanwhile. const replies = holdSendReplies(app); - await sendEdit(app, scope, "[mock:wait-start] edited message", "first message"); + await sendEdit(app, "[mock:wait-start] edited message", "first message"); + // The unsent draft is back once the edit is accepted; the user replaces it. + await app.chat.expectInputValue("unsent draft", LOAD_TOLERANT_WAIT.timeout); const save = await holdNextSendBeforeClear(app); await app.chat.typeWithoutSending("follow-up"); pressEnterInComposer(app, "follow-up"); - // The edit completes while the follow-up waits: the pre-edit draft comes back. + // The edit completes while the follow-up waits: nothing is restored into the composer. replies.release(); + // Edits work again once the edit send settled. await waitFor( - () => expect(getDraftStore().getText(scope)).toBe("unsent draft\n\nfollow-up"), + () => expect(rowEditButton(app, "edited message")?.disabled).toBe(false), LOAD_TOLERANT_WAIT ); + expect(getDraftStore().getText(scope)).toBe("follow-up"); save.release(); await waitFor( @@ -561,7 +835,7 @@ describe("Edit sends racing newer composer input (#5226)", () => { "Mock response: follow-up", LOAD_TOLERANT_WAIT.timeout ); - await expectUnsentDraftKept(app, scope); + await app.chat.expectInputValue("", LOAD_TOLERANT_WAIT.timeout); replies.spy.mockRestore(); save.spy.mockRestore(); } finally { @@ -582,7 +856,9 @@ describe("Edit sends racing newer composer input (#5226)", () => { await editRow(app, "first message"); const replies = holdSendReplies(app); - await sendEdit(app, scope, "[mock:wait-start] edited message", "first message"); + await sendEdit(app, "[mock:wait-start] edited message", "first message"); + // The pre-edit text never left the draft: it shows once the edit is accepted. + await app.chat.expectInputValue(restoredText, LOAD_TOLERANT_WAIT.timeout); // While the edit's stream starts, a queued message without notes goes back into the // composer: its note list is empty, and that is what the follow-up send captures. @@ -593,12 +869,14 @@ describe("Edit sends racing newer composer input (#5226)", () => { const save = await holdNextSendBeforeClear(app); pressEnterInComposer(app, "second follow-up"); - // The edit completes while the follow-up waits: its draft and note come back. + // The edit completes while the follow-up waits: its note comes back. (Its text never + // left the draft; sending the follow-up above replaced it.) replies.release(); await waitFor( - () => expect(getDraftStore().getText(scope)).toBe(`${restoredText}\n\nsecond follow-up`), + () => expect(reviewPanelNotes(app).join("\n")).toContain("pre-edit note"), LOAD_TOLERANT_WAIT ); + expect(getDraftStore().getText(scope)).toBe("second follow-up"); save.release(); await waitFor( @@ -610,7 +888,7 @@ describe("Edit sends racing newer composer input (#5226)", () => { "Mock response: second follow-up", LOAD_TOLERANT_WAIT.timeout ); - await app.chat.expectInputValue(restoredText, LOAD_TOLERANT_WAIT.timeout); + await app.chat.expectInputValue("", LOAD_TOLERANT_WAIT.timeout); await waitFor( () => expect(reviewPanelNotes(app).join("\n")).toContain("pre-edit note"), LOAD_TOLERANT_WAIT @@ -631,7 +909,6 @@ describe("Edit refused because history changed (B8)", () => { test("the failure alert goes away once the reviewed edit is sent", async () => { const app = await createAppHarness({ branchPrefix: "edit-history-changed-alert" }); try { - const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; await app.chat.send("first message"); await app.chat.expectTranscriptContains( "Mock response: first message", @@ -645,7 +922,7 @@ describe("Edit refused because history changed (B8)", () => { const sendSpy = jest .spyOn(workspaceService, "sendMessage") .mockResolvedValueOnce(Err({ type: "history-changed" })); - getDraftStore().setText(scope, "edited message"); + typeIntoEdit(editTextarea(app)!, "edited message"); await waitFor(() => expect(editTextarea(app)?.value).toBe("edited message")); fireEvent.keyDown(editTextarea(app)!, { key: "Enter" }); await waitFor( @@ -673,3 +950,762 @@ describe("Edit refused because history changed (B8)", () => { } }, 120_000); }); + +/** A second workspace of the same project, known to the sidebar. */ +async function addOtherWorkspace(app: AppHarness, prefix: string) { + const created = await app.env.orpc.workspace.create({ + projectPath: app.repoPath, + branchName: generateBranchName(prefix), + trunkBranch: await detectDefaultTrunkBranch(app.repoPath), + }); + if (!created.success) throw new Error(created.error); + workspaceStore.addWorkspace(created.metadata); + return created.metadata; +} + +/** Show `other`, then the harness workspace again: the composer of A unmounts and remounts. */ +async function switchAwayAndBack(app: AppHarness, other: { id: string; name: string }) { + await showWorkspace(app, other.id, other.name); + await showWorkspace(app, app.workspaceId, app.metadata.name); +} + +/** + * Visit `other` once and send a message there, then show the harness workspace again: with + * cached rows in both, ChatPane stays mounted across later switches (no loading placeholder). + */ +async function visitWithMessage(app: AppHarness, other: { id: string; name: string }) { + await showWorkspace(app, other.id, other.name); + const otherChat = new ChatHarness(app.view.container, other.id); + await otherChat.send("other message"); + await otherChat.expectTranscriptContains( + "Mock response: other message", + LOAD_TOLERANT_WAIT.timeout + ); + await otherChat.expectStreamComplete(); + await showWorkspace(app, app.workspaceId, app.metadata.name); +} + +/** The history id of the user row that shows `content`. */ +function userRowId(workspaceId: string, content: string) { + const row = useWorkspaceStoreRaw() + .getWorkspaceState(workspaceId) + .messages.find((message) => message.type === "user" && message.content === content); + const id = row?.type === "user" ? row.historyId : undefined; + if (!id) throw new Error(`No user row "${content}"`); + return id; +} + +type SendSpy = jest.SpyInstance< + ReturnType, + Parameters +>; + +/** + * Hold edit sends (requests with an `editMessageId`) before the backend sees them, until + * released; other sends go through. `refuse`: the held edit is then refused with this error. + * `replyOnly`: the backend takes the edit at once (its row is replaced) and only the reply waits. + */ +function holdEditSends( + app: AppHarness, + refuse?: { type: "history-changed" | "unknown" }, + replyOnly = false +) { + const workspaceService = app.env.services.workspaceService; + const realSend = workspaceService.sendMessage.bind(workspaceService); + let release: () => void = () => undefined; + const gate = new Promise((resolve) => { + release = resolve; + }); + const spy: SendSpy = jest + .spyOn(workspaceService, "sendMessage") + .mockImplementation(async (...args: Parameters) => { + if (args[2].editMessageId === undefined) return realSend(...args); + const reply = replyOnly ? await realSend(...args) : null; + await gate; + if (reply) return reply; + if (!refuse) return realSend(...args); + return refuse.type === "unknown" + ? Err({ type: "unknown", raw: "refused" }) + : Err({ type: "history-changed" }); + }); + return { release, spy }; +} + +const editRequests = (spy: SendSpy, editId: string) => + spy.mock.calls.filter(([, , options]) => options.editMessageId === editId).length; + +/** How many times `needle` occurs in `text`. */ +const occurrences = (text: string, needle: string) => text.split(needle).length - 1; + +/** How many notes in the composer's review panel show `note`. */ +const notesShowing = (app: AppHarness, note: string) => + reviewPanelNotes(app).filter((text) => text.includes(note)).length; + +/** Send `text` with one attached note, and wait until its reply is complete. */ +async function sendWithNote(app: AppHarness, text: string, id: string, note: string) { + await attachStoreReview(app, id, note); + await waitFor(() => expect(composerText(app)).toContain(note), LOAD_TOLERANT_WAIT); + await app.chat.send(text); + await app.chat.expectTranscriptContains(mockReply(text, true), LOAD_TOLERANT_WAIT.timeout); + await app.chat.expectStreamComplete(); +} + +/** + * While an edit send is in flight, put a queued message (`text` with `note`) back into the + * composer with its Edit action: the note joins the composer's own note list during the send. + * The review panel hides while an edit send is in flight, so the restored text is the signal. + * `holdBusy`: the workspace is idle, so a held stream keeps it busy while `text` queues. + */ +async function restoreQueuedNoteDuringEditSend( + app: AppHarness, + text: string, + note: string, + holdBusy: boolean +) { + const session = app.env.services.workspaceService.getOrCreateSession(app.workspaceId); + const options = { model: "openai:gpt-5.2", agentId: "exec" } as const; + const holding = holdBusy + ? app.env.orpc.workspace.sendMessage({ + workspaceId: app.workspaceId, + message: "[mock:wait-start] hold the workspace busy", + options, + }) + : null; + await waitFor(() => expect(session.isBusy()).toBe(true), LOAD_TOLERANT_WAIT); + await app.env.orpc.workspace.sendMessage({ + workspaceId: app.workspaceId, + message: text, + options: { ...options, muxMetadata: { type: "normal", reviews: [review(note)] } }, + }); + await waitFor(() => expect(session.hasQueuedMessages()).toBe(true), LOAD_TOLERANT_WAIT); + await editQueuedMessage(app); + await waitFor(() => { + const values = [ + ...app.view.container.querySelectorAll( + '[data-component="ChatInputSection"] textarea' + ), + ].map((textarea) => textarea.value); + expect(values.some((value) => value.includes(text))).toBe(true); + }, LOAD_TOLERANT_WAIT); + if (holding) { + app.env.services.aiService.releaseMockStreamStartGate(app.workspaceId); + await holding; + await app.chat.expectStreamComplete(); + } +} + +/** The composer's Send button. */ +const sendButton = (app: AppHarness) => + app.view.container.querySelector( + '[data-component="ChatInputSection"] button[aria-label="Send message"]' + ); + +/** + * Serve the workspace's state through `view` (one view object per store state, as + * useSyncExternalStore needs a stable snapshot): a replay as the transcript shows it. + */ +function replayView(app: AppHarness, view: (state: WorkspaceState) => WorkspaceState) { + // eslint-disable-next-line @typescript-eslint/unbound-method -- called below with the store as `this` + const realGetState = WorkspaceStore.prototype.getWorkspaceState; + const views = new WeakMap(); + return jest.spyOn(WorkspaceStore.prototype, "getWorkspaceState").mockImplementation(function ( + this: WorkspaceStore, + workspaceId: string + ) { + const state = realGetState.call(this, workspaceId); + if (workspaceId !== app.workspaceId) return state; + let shown = views.get(state); + if (!shown) { + shown = view(state); + views.set(state, shown); + } + return shown; + }); +} + +/** Re-render ChatPane with the store's current view, and let its effects run. */ +async function rerenderTranscript(app: AppHarness) { + await act(async () => { + useWorkspaceStoreRaw().bumpState(app.workspaceId); + await new Promise((resolve) => setTimeout(resolve, 500)); + }); +} + +// A workspace switch remounts the composer (ChatPane keys it by workspace). The switch ends the +// open edit, as on main, and the edit's text, files and notes move once into that workspace's +// draft, after the unsent draft (#5808). No edit state survives a composer remount. +describe("A workspace switch ends an open edit (#5808)", () => { + beforeAll(async () => { + await preloadTestModules(); + }); + + test("an edit sent before a workspace switch is closed on return and is sent only once", async () => { + const app = await createAppHarness({ branchPrefix: "switch-edit-sent-once" }); + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + const other = await addOtherWorkspace(app, "switch-edit-sent-once-other"); + const textarea = await startEditWithUnsentDraft(app, scope); + const editId = userRowId(app.workspaceId, "first message"); + const sends = holdEditSends(app); + typeIntoEdit(textarea, "edited message"); + await waitFor(() => expect(textarea.value).toBe("edited message")); + fireEvent.keyDown(textarea, { key: "Enter" }); + await waitFor(() => expect(editRequests(sends.spy, editId)).toBe(1), LOAD_TOLERANT_WAIT); + + await switchAwayAndBack(app, other); + // No edit is open on return, so nothing can send the edit a second time. + expect(editTextarea(app)).toBeNull(); + await app.chat.expectInputValue("unsent draft", LOAD_TOLERANT_WAIT.timeout); + + sends.release(); + await app.chat.expectTranscriptContains("edited message", LOAD_TOLERANT_WAIT.timeout); + await app.chat.expectStreamComplete(); + expect(editRequests(sends.spy, editId)).toBe(1); + expect(editTextarea(app)).toBeNull(); + await expectUnsentDraftKept(app, scope); + sends.spy.mockRestore(); + } finally { + await app.dispose(); + } + }, 120_000); + + test("an edit accepted after a workspace switch brings back only the pre-edit notes", async () => { + const app = await createAppHarness({ branchPrefix: "switch-edit-accepted-notes" }); + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + const other = await addOtherWorkspace(app, "switch-edit-accepted-notes-other"); + await sendWithNote(app, "first message", "review-row", "row note"); + // The pre-edit draft has its own note list (P). + await restoreQueuedMessageWithNote(app, scope, "pre-edit note"); + await editRow(app, "first message"); + await waitFor(() => expect(notesShowing(app, "row note")).toBe(1), LOAD_TOLERANT_WAIT); + + const replies = holdEditSends(app, undefined, true); + await sendEdit(app, "[mock:wait-start] edited message", "first message"); + // During the send a queued message with note R goes back into the composer. + await restoreQueuedNoteDuringEditSend(app, "second follow-up", "restored note", false); + + // The edit is accepted after its composer unmounted. + await switchAwayAndBack(app, other); + replies.release(); + app.env.services.aiService.releaseMockStreamStartGate(app.workspaceId); + await app.chat.expectStreamComplete(); + await waitFor(() => { + expect(notesShowing(app, "pre-edit note")).toBe(1); + expect(notesShowing(app, "restored note")).toBe(1); + }, LOAD_TOLERANT_WAIT); + + // The next message carries P and R once each, and never the edit's own note. + await app.chat.send("final follow-up"); + const sent = await waitFor(() => { + const call = replies.spy.mock.calls.find(([, message]) => + message.endsWith("final follow-up") + ); + if (!call) throw new Error("final follow-up not sent"); + return call[1]; + }, LOAD_TOLERANT_WAIT); + expect(occurrences(sent, "pre-edit note")).toBe(1); + expect(occurrences(sent, "restored note")).toBe(1); + expect(occurrences(sent, "row note")).toBe(0); + await app.chat.expectStreamComplete(); + replies.spy.mockRestore(); + } finally { + await app.dispose(); + } + }, 120_000); + + test("an edit whose history-changed refresh failed ends on a switch, with its text after the unsent draft", async () => { + const app = await createAppHarness({ branchPrefix: "switch-edit-refresh-failed" }); + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + const other = await addOtherWorkspace(app, "switch-edit-refresh-failed-other"); + const textarea = await startEditWithUnsentDraft(app, scope); + const sendSpy = jest + .spyOn(app.env.services.workspaceService, "sendMessage") + .mockResolvedValueOnce(Err({ type: "history-changed" })); + const refreshSpy = jest + .spyOn(WorkspaceStore.prototype, "requestTranscriptRefresh") + .mockResolvedValue({ kind: "failed", error: "refresh unavailable" }); + typeIntoEdit(textarea, "edited message"); + await waitFor(() => expect(textarea.value).toBe("edited message")); + fireEvent.keyDown(textarea, { key: "Enter" }); + await waitFor( + () => expect(composerText(app)).toContain("transcript refresh failed"), + LOAD_TOLERANT_WAIT + ); + + await switchAwayAndBack(app, other); + expect(editTextarea(app)).toBeNull(); + expect(composerText(app)).not.toContain("refreshing transcript"); + expect(composerText(app)).not.toContain("transcript refresh failed"); + await waitFor( + () => + expect(messageTextarea(app).value).toBe(joinDraftText("unsent draft", "edited message")), + LOAD_TOLERANT_WAIT + ); + expect(getDraftStore().getText(scope)).toBe(messageTextarea(app).value); + await waitFor(() => expect(sendButton(app)?.disabled).toBe(false), LOAD_TOLERANT_WAIT); + refreshSpy.mockRestore(); + sendSpy.mockRestore(); + } finally { + await app.dispose(); + } + }, 120_000); + + // An edit cannot add files, so its file change is removing one of the message's files. + test("a workspace switch ends an open edit and keeps its text and files after the unsent draft, once", async () => { + const app = await createAppHarness({ branchPrefix: "switch-edit-moves-contents" }); + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + const other = await addOtherWorkspace(app, "switch-edit-moves-contents-other"); + const fileInput = await waitFor(() => { + const element = app.view.container.querySelector( + '[data-component="ChatInputSection"] input[type="file"]' + ); + if (!element) throw new Error("File input not found"); + return element; + }, LOAD_TOLERANT_WAIT); + fireEvent.change(fileInput, { + target: { + files: [ + new File(["# kept"], "edit-kept.md", { type: "text/markdown" }), + new File(["# removed"], "edit-removed.md", { type: "text/markdown" }), + ], + }, + }); + await waitFor(() => { + expect(composerText(app)).toContain("edit-kept.md"); + expect(composerText(app)).toContain("edit-removed.md"); + }, LOAD_TOLERANT_WAIT); + const textarea = await startEditWithUnsentDraft(app, scope); + await waitFor( + () => expect(composerText(app)).toContain("edit-removed.md"), + LOAD_TOLERANT_WAIT + ); + typeIntoEdit(textarea, "edited before switch"); + await waitFor(() => expect(textarea.value).toBe("edited before switch")); + const removeButton = [ + ...app.view.container.querySelectorAll( + '[data-component="ChatInputSection"] button[aria-label="Remove attachment"]' + ), + ].find((button) => button.parentElement?.textContent?.includes("edit-removed.md")); + if (!removeButton) throw new Error("Remove button of edit-removed.md not found"); + fireEvent.click(removeButton); + await waitFor( + () => expect(composerText(app)).not.toContain("edit-removed.md"), + LOAD_TOLERANT_WAIT + ); + + await switchAwayAndBack(app, other); + await switchAwayAndBack(app, other); + expect(editTextarea(app)).toBeNull(); + const expected = joinDraftText("unsent draft", "edited before switch"); + await waitFor(() => expect(messageTextarea(app).value).toBe(expected), LOAD_TOLERANT_WAIT); + const names = (attachments: { filename?: string; id: string }[]) => + attachments.map((attachment) => attachment.filename ?? attachment.id); + expect(getDraftStore().getText(scope)).toBe(expected); + expect(names(getDraftStore().getView(scope).attachments)).toEqual([ + "unsent.txt", + "edit-kept.md", + ]); + await getDraftStore().flush(scope); + const saved = await app.env.services.draftService.get(scope); + expect(saved.text).toBe(expected); + expect(saved.attachments).toHaveLength(2); + } finally { + await app.dispose(); + } + }, 120_000); + + test("an edit refused after a workspace switch keeps its text, files and notes in that workspace's draft, once", async () => { + const app = await createAppHarness({ branchPrefix: "switch-edit-refused" }); + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + const other = await addOtherWorkspace(app, "switch-edit-refused-other"); + const otherScope: DraftScope = { kind: "workspace", workspaceId: other.id }; + await attachComposerFile(app, "edit-file.md"); + const textarea = await startEditWithUnsentDraft(app, scope, "row note"); + const sends = holdEditSends(app, { type: "history-changed" }); + const refreshSpy = jest.spyOn(WorkspaceStore.prototype, "requestTranscriptRefresh"); + typeIntoEdit(textarea, "edited message"); + await waitFor(() => expect(textarea.value).toBe("edited message")); + fireEvent.keyDown(textarea, { key: "Enter" }); + await waitFor( + () => expect(sends.spy.mock.calls.some(([, , o]) => o.editMessageId)).toBe(true), + LOAD_TOLERANT_WAIT + ); + await restoreQueuedNoteDuringEditSend(app, "queued with note", "restored note", true); + + // The refusal arrives while the other workspace is shown. + await showWorkspace(app, other.id, other.name); + sends.release(); + await waitFor( + () => expect(getDraftStore().getText(scope)).toContain("edited message"), + LOAD_TOLERANT_WAIT + ); + await showWorkspace(app, app.workspaceId, app.metadata.name); + + expect(editTextarea(app)).toBeNull(); + const expected = joinDraftText("unsent draft", "edited message", "queued with note"); + await waitFor(() => expect(messageTextarea(app).value).toBe(expected), LOAD_TOLERANT_WAIT); + expect(getDraftStore().getText(scope)).toBe(expected); + const names = (attachments: { filename?: string; id: string }[]) => + attachments.map((attachment) => attachment.filename ?? attachment.id); + expect(names(getDraftStore().getView(scope).attachments)).toEqual([ + "unsent.txt", + "edit-file.md", + ]); + await waitFor(() => { + expect(notesShowing(app, "row note")).toBe(1); + expect(notesShowing(app, "restored note")).toBe(1); + }, LOAD_TOLERANT_WAIT); + expect(getDraftStore().getText(otherScope)).toBe(""); + // The unmounted composer starts no transcript refresh for its dead edit. + expect(refreshSpy.mock.calls.filter(([id]) => id === app.workspaceId)).toHaveLength(0); + refreshSpy.mockRestore(); + sends.spy.mockRestore(); + } finally { + await app.dispose(); + } + }, 120_000); + + test("a workspace switch moves an idle edit's notes to the attached notes, once", async () => { + const app = await createAppHarness({ branchPrefix: "switch-edit-moves-notes" }); + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + const other = await addOtherWorkspace(app, "switch-edit-moves-notes-other"); + const textarea = await startEditWithUnsentDraft(app, scope, "row note"); + await waitFor(() => expect(notesShowing(app, "row note")).toBe(1), LOAD_TOLERANT_WAIT); + typeIntoEdit(textarea, "edited message"); + await waitFor(() => expect(textarea.value).toBe("edited message")); + + await switchAwayAndBack(app, other); + await switchAwayAndBack(app, other); + expect(editTextarea(app)).toBeNull(); + await waitFor(() => expect(notesShowing(app, "row note")).toBe(1), LOAD_TOLERANT_WAIT); + expect(getDraftStore().getText(scope)).toBe(joinDraftText("unsent draft", "edited message")); + } finally { + await app.dispose(); + } + }, 120_000); + + // Main keeps one edit slot in ChatPane: a late cancel from A's unmounted composer would + // close the edit the user opened in B meanwhile. + test("an edit accepted after a switch does not close an edit opened in the other workspace", async () => { + const app = await createAppHarness({ branchPrefix: "switch-edit-keeps-other-edit" }); + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + const other = await addOtherWorkspace(app, "switch-edit-keeps-other-edit-other"); + await visitWithMessage(app, other); + + const textarea = await startEditWithUnsentDraft(app, scope); + const editId = userRowId(app.workspaceId, "first message"); + const sends = holdEditSends(app); + typeIntoEdit(textarea, "edited message"); + await waitFor(() => expect(textarea.value).toBe("edited message")); + fireEvent.keyDown(textarea, { key: "Enter" }); + await waitFor(() => expect(editRequests(sends.spy, editId)).toBe(1), LOAD_TOLERANT_WAIT); + + await showWorkspace(app, other.id, other.name); + await editRow(app, "other message"); + typeIntoEdit(editTextarea(app)!, "edited other message"); + await waitFor(() => expect(editTextarea(app)?.value).toBe("edited other message")); + + sends.release(); + // Wait for A's held edit send to return, then for its completion to run. + await Promise.all(sends.spy.mock.results.map((result) => result.value as Promise)); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 500)); + }); + expect(editTextarea(app)?.value).toBe("edited other message"); + sends.spy.mockRestore(); + } finally { + await app.dispose(); + } + }, 120_000); + + test("an editing /compact accepted after a switch never shows its command text in the draft", async () => { + const app = await createAppHarness({ branchPrefix: "switch-edit-compact" }); + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + const other = await addOtherWorkspace(app, "switch-edit-compact-other"); + const textarea = await startEditWithUnsentDraft(app, scope); + // Hold the compaction request: the command keeps its text in the edit until accepted. + const workspaceService = app.env.services.workspaceService; + const realSend = workspaceService.sendMessage.bind(workspaceService); + let releaseSend: () => void = () => undefined; + const sendGate = new Promise((resolve) => { + releaseSend = resolve; + }); + const sendSpy = jest + .spyOn(workspaceService, "sendMessage") + .mockImplementation(async (...args: Parameters) => { + await sendGate; + return realSend(...args); + }); + typeIntoEdit(textarea, "/compact -t 500"); + await waitFor(() => expect(textarea.value).toBe("/compact -t 500")); + fireEvent.keyDown(textarea, { key: "Enter" }); + await waitFor(() => expect(sendSpy).toHaveBeenCalled(), LOAD_TOLERANT_WAIT); + + await switchAwayAndBack(app, other); + expect(editTextarea(app)).toBeNull(); + expect(getDraftStore().getText(scope)).toBe("unsent draft"); + releaseSend(); + await app.chat.expectStreamComplete(60_000); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 500)); + }); + await expectUnsentDraftKept(app, scope); + sendSpy.mockRestore(); + } finally { + await app.dispose(); + } + }, 120_000); + + test("under StrictMode an accepted edit closes, and a switch moves an idle edit once", async () => { + const app = await createAppHarness({ branchPrefix: "switch-edit-strict", strictMode: true }); + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + const other = await addOtherWorkspace(app, "switch-edit-strict-other"); + const textarea = await startEditWithUnsentDraft(app, scope); + typeIntoEdit(textarea, "edited message"); + await waitFor(() => expect(textarea.value).toBe("edited message")); + fireEvent.keyDown(textarea, { key: "Enter" }); + await app.chat.expectTranscriptContains("edited message", LOAD_TOLERANT_WAIT.timeout); + await app.chat.expectStreamComplete(); + await waitFor(() => expect(editTextarea(app)).toBeNull(), LOAD_TOLERANT_WAIT); + await expectUnsentDraftKept(app, scope); + + await editRow(app, "edited message"); + typeIntoEdit(editTextarea(app)!, "edited again"); + await waitFor(() => expect(editTextarea(app)?.value).toBe("edited again")); + await switchAwayAndBack(app, other); + expect(editTextarea(app)).toBeNull(); + const expected = joinDraftText("unsent draft", "edited again"); + await waitFor(() => expect(messageTextarea(app).value).toBe(expected), LOAD_TOLERANT_WAIT); + expect(getDraftStore().getText(scope)).toBe(expected); + } finally { + await app.dispose(); + } + }, 120_000); + + test("an edit refused after its row was deleted keeps its text after the unsent draft, once", async () => { + const app = await createAppHarness({ branchPrefix: "edit-refused-row-deleted" }); + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + const textarea = await startEditWithUnsentDraft(app, scope); + const sends = holdEditSends(app, { type: "unknown" }); + typeIntoEdit(textarea, "edited message"); + await waitFor(() => expect(textarea.value).toBe("edited message")); + fireEvent.keyDown(textarea, { key: "Enter" }); + await waitFor( + () => expect(sends.spy.mock.calls.some(([, , o]) => o.editMessageId)).toBe(true), + LOAD_TOLERANT_WAIT + ); + const cleared = await app.env.services.workspaceService.truncateHistory(app.workspaceId); + expect(cleared.success).toBe(true); + await waitFor(() => expect(editTextarea(app)).toBeNull(), LOAD_TOLERANT_WAIT); + + sends.release(); + await expectEditKeptAsDraft(app, scope); + expect(getDraftStore().getText(scope)).toBe(joinDraftText("unsent draft", "edited message")); + sends.spy.mockRestore(); + } finally { + await app.dispose(); + } + }, 120_000); + + test("an edit whose row is deleted keeps its notes as attached notes after a switch", async () => { + const app = await createAppHarness({ branchPrefix: "edit-row-deleted-notes" }); + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + const other = await addOtherWorkspace(app, "edit-row-deleted-notes-other"); + const textarea = await startEditWithUnsentDraft(app, scope, "row note"); + typeIntoEdit(textarea, "edited message"); + await waitFor(() => expect(textarea.value).toBe("edited message")); + const cleared = await app.env.services.workspaceService.truncateHistory(app.workspaceId); + expect(cleared.success).toBe(true); + await waitFor(() => expect(editTextarea(app)).toBeNull(), LOAD_TOLERANT_WAIT); + await expectEditKeptAsDraft(app, scope); + + await switchAwayAndBack(app, other); + await switchAwayAndBack(app, other); + await waitFor(() => expect(notesShowing(app, "row note")).toBe(1), LOAD_TOLERANT_WAIT); + } finally { + await app.dispose(); + } + }, 120_000); + + test("a second Edit with notes, then Cancel, shows the first edit's notes once", async () => { + const app = await createAppHarness({ branchPrefix: "second-edit-notes-cancel" }); + try { + await sendWithNote(app, "earlier message", "review-earlier", "second row note"); + await sendWithNote(app, "first message", "review-first", "first row note"); + await editRow(app, "first message"); + typeIntoEdit(editTextarea(app)!, "edited message"); + await waitFor(() => expect(editTextarea(app)?.value).toBe("edited message")); + + await editRow(app, "earlier message"); + await waitFor(() => expect(notesShowing(app, "second row note")).toBe(1), LOAD_TOLERANT_WAIT); + fireEvent.keyDown(editTextarea(app)!, { key: "Escape" }); + await waitFor(() => expect(editTextarea(app)).toBeNull(), LOAD_TOLERANT_WAIT); + await waitFor(() => expect(notesShowing(app, "first row note")).toBe(1), LOAD_TOLERANT_WAIT); + expect(notesShowing(app, "second row note")).toBe(0); + + // The note is sent once, and then it is done: it does not stay attached. + const sendSpy = jest.spyOn(app.env.services.workspaceService, "sendMessage"); + await app.chat.send("follow-up"); + const sent = await waitFor(() => { + const call = sendSpy.mock.calls.find(([, message]) => message.endsWith("follow-up")); + if (!call) throw new Error("follow-up not sent"); + return call[1]; + }, LOAD_TOLERANT_WAIT); + expect(occurrences(sent, "first row note")).toBe(1); + await app.chat.expectStreamComplete(); + // Read the store, not the panel: the panel hides while the send is in flight. + await waitFor( + () => + expect( + getReviewStateStore() + .getAttachedReviews(app.workspaceId) + .filter((attached) => attached.data.userNote === "first row note") + ).toHaveLength(0), + LOAD_TOLERANT_WAIT + ); + sendSpy.mockRestore(); + } finally { + await app.dispose(); + } + }, 120_000); + + test("an edit cancelled while its send is still preparing sends nothing", async () => { + const app = await createAppHarness({ branchPrefix: "edit-cancel-preparing" }); + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + const textarea = await startEditWithUnsentDraft(app, scope); + const sendSpy: SendSpy = jest.spyOn(app.env.services.workspaceService, "sendMessage"); + const save = await holdNextSendBeforeClear(app); + typeIntoEdit(textarea, "edited message"); + await waitFor(() => expect(textarea.value).toBe("edited message")); + fireEvent.keyDown(textarea, { key: "Enter" }); + const composer = app.view.container.querySelector( + '[data-component="ChatInputSection"]' + )!; + fireEvent.click(within(composer).getByRole("button", { name: "Cancel" })); + await waitFor(() => expect(editTextarea(app)).toBeNull(), LOAD_TOLERANT_WAIT); + + save.release(); + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 1_000)); + }); + expect(sendSpy.mock.calls.filter(([, , o]) => o.editMessageId)).toHaveLength(0); + // The send settled: Edit works again. + await waitFor( + () => expect(rowEditButton(app, "first message")?.disabled).toBe(false), + LOAD_TOLERANT_WAIT + ); + await expectUnsentDraftKept(app, scope); + save.spy.mockRestore(); + sendSpy.mockRestore(); + } finally { + await app.dispose(); + } + }, 120_000); + + // A replay of the shown workspace can empty its rows while ChatPane stays mounted: an open + // edit must not close because its row is briefly missing. + test("an open edit stays open while its workspace replays and has not caught up", async () => { + const app = await createAppHarness({ branchPrefix: "edit-replay-not-caught-up" }); + let stateSpy: jest.SpyInstance | null = null; + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + const textarea = await startEditWithUnsentDraft(app, scope); + typeIntoEdit(textarea, "edited message"); + await waitFor(() => expect(textarea.value).toBe("edited message")); + stateSpy = replayView(app, (state) => ({ + ...state, + isTranscriptCaughtUp: false, + messages: [], + })); + await rerenderTranscript(app); + expect(editTextarea(app)?.value).toBe("edited message"); + expect(getDraftStore().getText(scope)).toBe("unsent draft"); + } finally { + stateSpy?.mockRestore(); + await app.dispose(); + } + }, 120_000); + + // A windowed replay (#4961) can load only the newest rows: the edit's row is then older + // history, not gone. + test("an open edit stays open when a replay leaves its row outside the window", async () => { + const app = await createAppHarness({ branchPrefix: "edit-replay-windowed" }); + let stateSpy: jest.SpyInstance | null = null; + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + const textarea = await startEditWithUnsentDraft(app, scope); + const editId = userRowId(app.workspaceId, "first message"); + typeIntoEdit(textarea, "edited message"); + await waitFor(() => expect(textarea.value).toBe("edited message")); + stateSpy = replayView(app, (state) => ({ + ...state, + hasOlderHistory: true, + messages: state.messages.filter((row) => !("historyId" in row) || row.historyId !== editId), + })); + await rerenderTranscript(app); + expect(editTextarea(app)?.value).toBe("edited message"); + expect(getDraftStore().getText(scope)).toBe("unsent draft"); + } finally { + stateSpy?.mockRestore(); + await app.dispose(); + } + }, 120_000); + + // The switch, not the replay, decides: the edit ends even though the returning replay keeps + // its row in older history, and its text joins the draft once. + test("a workspace switch ends an open edit whose row a replay leaves outside the window", async () => { + const app = await createAppHarness({ branchPrefix: "switch-edit-row-windowed" }); + let stateSpy: jest.SpyInstance | null = null; + try { + const scope: DraftScope = { kind: "workspace", workspaceId: app.workspaceId }; + const other = await addOtherWorkspace(app, "switch-edit-row-windowed-other"); + // ChatPane stays mounted across the switches: the switch itself must end the edit. + await visitWithMessage(app, other); + const textarea = await startEditWithUnsentDraft(app, scope); + const editId = userRowId(app.workspaceId, "first message"); + typeIntoEdit(textarea, "edited before switch"); + await waitFor(() => expect(textarea.value).toBe("edited before switch")); + await showWorkspace(app, other.id, other.name); + stateSpy = replayView(app, (state) => ({ + ...state, + hasOlderHistory: true, + messages: state.messages.filter((row) => !("historyId" in row) || row.historyId !== editId), + })); + await showWorkspace(app, app.workspaceId, app.metadata.name); + await waitFor( + () => + expect( + useWorkspaceStoreRaw().getWorkspaceState(app.workspaceId).isTranscriptCaughtUp + ).toBe(true), + LOAD_TOLERANT_WAIT + ); + await rerenderTranscript(app); + expect(editTextarea(app)).toBeNull(); + const expected = joinDraftText("unsent draft", "edited before switch"); + await waitFor(() => expect(messageTextarea(app).value).toBe(expected), LOAD_TOLERANT_WAIT); + await switchAwayAndBack(app, other); + await rerenderTranscript(app); + expect(editTextarea(app)).toBeNull(); + expect(getDraftStore().getText(scope)).toBe(expected); + expect( + getDraftStore() + .getView(scope) + .attachments.map(({ id }) => id) + ).toEqual(["file-unsent"]); + } finally { + stateSpy?.mockRestore(); + await app.dispose(); + } + }, 120_000); +}); diff --git a/tests/ui/harness/createAppHarness.ts b/tests/ui/harness/createAppHarness.ts index 238daf8f66..62d506b5a7 100644 --- a/tests/ui/harness/createAppHarness.ts +++ b/tests/ui/harness/createAppHarness.ts @@ -54,6 +54,8 @@ export async function createAppHarness(options?: { * workspace-scoped persisted state (e.g. draft attachments). */ beforeRender?: (workspaceId: string) => void; + /** Render the app under React StrictMode (dev builds do). */ + strictMode?: boolean; }): Promise { const repoPath = await createTempGitRepo(); const env = await createTestEnvironment(); @@ -92,7 +94,7 @@ export async function createAppHarness(options?: { cleanupDom = installDom(); options?.beforeRender?.(workspaceId); - view = renderApp({ apiClient: env.orpc, metadata }); + view = renderApp({ apiClient: env.orpc, metadata, strictMode: options?.strictMode }); await setupWorkspaceView(view, metadata, workspaceId); await waitForWorkspaceChatToRender(view.container); diff --git a/tests/ui/renderReviewPanel.tsx b/tests/ui/renderReviewPanel.tsx index 43f6ab6052..0ab6971ebb 100644 --- a/tests/ui/renderReviewPanel.tsx +++ b/tests/ui/renderReviewPanel.tsx @@ -1,4 +1,5 @@ import { render, type RenderResult, waitFor } from "@testing-library/react"; +import { StrictMode } from "react"; import { AppLoader } from "@/browser/components/AppLoader/AppLoader"; import type { APIClient } from "@/browser/contexts/API"; @@ -8,6 +9,8 @@ interface RenderReviewPanelParams { apiClient: APIClient; /** Metadata for the workspace to select (optional - app can render without a workspace) */ metadata?: FrontendWorkspaceMetadata; + /** Render under React StrictMode, as dev builds do (main.tsx). */ + strictMode?: boolean; } export interface RenderedApp extends RenderResult { @@ -34,7 +37,8 @@ export function renderReviewPanel(props: RenderReviewPanelParams): RenderedApp { * This exercises the real component tree, providers, and state management. */ export function renderApp(props: RenderReviewPanelParams): RenderedApp { - const result = render(); + const app = ; + const result = render(props.strictMode ? {app} : app); return { ...result,