diff --git a/ui/src/components/IssueChatThread.test.tsx b/ui/src/components/IssueChatThread.test.tsx index ba5bec1b787..38a07c97c48 100644 --- a/ui/src/components/IssueChatThread.test.tsx +++ b/ui/src/components/IssueChatThread.test.tsx @@ -15,7 +15,7 @@ import { MemoryRouter } from "react-router-dom"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { Agent } from "@paperclipai/shared"; import { CommentSubmissionUnknownError } from "../lib/comment-submit-result"; -import { settleDraftSubmission } from "../lib/composer-draft"; +import { loadDraft, preserveDraftInTab, settleDraftSubmission } from "../lib/composer-draft"; import { IssueAssigneePausedNotice, IssueChatThread, @@ -380,6 +380,7 @@ describe("IssueChatThread", () => { document.body.appendChild(container); window.scrollTo = vi.fn(); localStorage.clear(); + sessionStorage.clear(); }); afterEach(() => { @@ -3380,44 +3381,99 @@ describe("IssueChatThread", () => { }, ); - it.each(["settled", "newer attempt"])("adopts another tab's draft after %s without overwriting it", async (outcome) => { - const key = "cross-tab-settled-submission"; - const attemptId = "aaf8228f-0be7-45ae-a104-6fbe0af6f1d3"; - localStorage.setItem(key, "Earlier message\n\nThis tab's next draft"); - localStorage.setItem(`${key}:submission:v1`, JSON.stringify({ version: 1, draftKey: key, attemptId, reviewed: false, nextDraftOffset: 17, submittedAttachmentIds: [] })); - const root = createRoot(container); - const element = (confirmed: boolean) => ( - - {}} draftKey={key} enableLiveTranscriptPolling={false} - /> - - ); - try { - await act(async () => root.render(element(false))); - expect(settleDraftSubmission(key, attemptId, "Newer text saved by another tab")).toBe(true); + it.each(["newer local text", "foreign pending receipt", "matching text with new attachments", "matching full text with new attachments", "own receipt with newer stored text", "unavailable recovery storage"])( + "preserves both tabs through settlement and reload: %s", + async (scenario) => { + const key = `cross-tab-preserved-${scenario}`; + const attemptId = "aaf8228f-0be7-45ae-a104-6fbe0af6f1d3"; const newerId = "baf8228f-0be7-45ae-a104-6fbe0af6f1d3"; - if (outcome === "newer attempt") localStorage.setItem(`${key}:submission:v1`, JSON.stringify({ version: 1, draftKey: key, attemptId: newerId, reviewed: false })); const attachmentId = "caf8228f-0be7-45ae-a104-6fbe0af6f1d3"; - localStorage.setItem(`${key}:attachments:v1`, JSON.stringify({ version: 1, draftKey: key, attachments: [{ attachmentId, name: "another-tab.txt", inline: false, contentPath: `/api/attachments/${attachmentId}/content` }] })); - await act(async () => root.render(element(true))); - expect(container.querySelector('textarea[aria-label="Issue chat editor"]')!.value).toBe("Newer text saved by another tab"); - if (outcome === "settled") { + const localText = "This tab's newer unsent draft"; + const otherText = scenario === "matching text with new attachments" ? localText : scenario === "matching full text with new attachments" ? `Earlier message\n\n${localText}` : "The other tab's unsent draft"; + localStorage.setItem(key, `Earlier message\n\n${localText}`); + localStorage.setItem(`${key}:submission:v1`, JSON.stringify({ version: 1, draftKey: key, attemptId, reviewed: false, nextDraftOffset: 17, submittedAttachmentIds: [] })); + const element = (confirmed: boolean, foreignConfirmed = false) => ( + + {}} draftKey={key} enableLiveTranscriptPolling={false} + /> + + ); + let root = createRoot(container); + const recoveryWrite = scenario === "unavailable recovery storage" + ? vi.spyOn(Object.getPrototypeOf(sessionStorage), "setItem").mockImplementation(() => { throw new Error("Storage full"); }) + : null; + try { + await act(async () => root.render(element(false))); + if (scenario === "own receipt with newer stored text") localStorage.setItem(key, `Earlier message\n\n${otherText}`); + else expect(settleDraftSubmission(key, attemptId, otherText)).toBe(true); + if (scenario === "foreign pending receipt") localStorage.setItem(`${key}:submission:v1`, JSON.stringify({ version: 1, draftKey: key, attemptId: newerId, reviewed: false, nextDraftOffset: 0, submittedAttachmentIds: [] })); + localStorage.setItem(`${key}:attachments:v1`, JSON.stringify({ version: 1, draftKey: key, attachments: [{ attachmentId, name: "another-tab.txt", inline: false, contentPath: `/api/attachments/${attachmentId}/content` }] })); + await act(async () => root.render(element(true))); + const editor = () => container.querySelector('textarea[aria-label="Issue chat editor"]')!; + expect(editor().value).toBe(localText); expect(container.textContent).not.toContain("We couldn’t confirm"); - expect(localStorage.getItem(`${key}:submission:v1`)).toBeNull(); - } else { - expect(container.textContent).toContain("We couldn’t confirm"); - expect(localStorage.getItem(`${key}:submission:v1`)).toContain(newerId); + if (scenario === "unavailable recovery storage") expect(container.textContent).toContain("copy your text before leaving"); + if (scenario.includes("with new attachments")) expect(container.textContent).toContain("another-tab.txt"); + if (scenario === "foreign pending receipt") { + localStorage.setItem(key, "The owning tab kept typing after the first receipt"); + await act(async () => root.render(element(true, true))); + expect(localStorage.getItem(`${key}:submission:v1`)).toContain(newerId); + } else expect(localStorage.getItem(`${key}:submission:v1`)).toBeNull(); + const storedText = localStorage.getItem(key); + expect(storedText).toBe(scenario === "foreign pending receipt" ? "The owning tab kept typing after the first receipt" : otherText); + expect(localStorage.getItem(`${key}:attachments:v1`)).toContain(attachmentId); + await act(async () => root.unmount()); + expect(localStorage.getItem(key)).toBe(storedText); + if (scenario === "unavailable recovery storage") return; + root = createRoot(container); + await act(async () => root.render(element(true, true))); + expect(editor().value).toBe(localText); + if (scenario.includes("with new attachments")) expect(container.textContent).toContain("another-tab.txt"); + expect(localStorage.getItem(key)).toBe(storedText); + expect(localStorage.getItem(`${key}:attachments:v1`)).toContain(attachmentId); + } finally { + if (container.childNodes.length) await act(async () => root.unmount()); + recoveryWrite?.mockRestore(); } - expect(container.textContent).toContain("another-tab.txt"); - expect(localStorage.getItem(`${key}:attachments:v1`)).toContain(attachmentId); - expect(localStorage.getItem(key)).toBe("Newer text saved by another tab"); - } finally { - await act(async () => root.unmount()); - } - expect(localStorage.getItem(key)).toBe("Newer text saved by another tab"); + }, + ); + + it.each(["ordinary", "recovered"])("preserves %s drafts when the same composer switches A to B to A before debounce", async (kind) => { + vi.useFakeTimers(); + const a = `navigation-a-${kind}`; + const b = `navigation-b-${kind}`; + const keys = kind === "recovered" + ? [preserveDraftInTab(a, "Draft A", []).key, preserveDraftInTab(b, "Draft B", []).key] + : [a, b]; + if (kind === "ordinary") { localStorage.setItem(a, "Draft A"); localStorage.setItem(b, "Draft B"); } + const root = createRoot(container); + const element = (draftKey: string) => ( + {}} draftKey={draftKey} enableLiveTranscriptPolling={false} /> + ); + const editor = () => container.querySelector('textarea[aria-label="Issue chat editor"]')!; + const type = (value: string) => act(() => { + Object.getOwnPropertyDescriptor(window.HTMLTextAreaElement.prototype, "value")!.set!.call(editor(), value); + editor().dispatchEvent(new Event("input", { bubbles: true })); + }); + try { + await act(async () => root.render(element(a))); + expect(editor().value).toBe("Draft A"); + type("Draft A typed just now"); + await act(async () => root.render(element(b))); + expect(editor().value).toBe("Draft B"); + expect(loadDraft(keys[0]!)).toBe("Draft A typed just now"); + type("Draft B typed just now"); + await act(async () => root.render(element(a))); + expect(editor().value).toBe("Draft A typed just now"); + expect(loadDraft(keys[1]!)).toBe("Draft B typed just now"); + } finally { await act(async () => root.unmount()); } }); it("stores and restores the composer draft per issue key", () => { diff --git a/ui/src/components/IssueChatThread.tsx b/ui/src/components/IssueChatThread.tsx index 7bf758080f3..6db7961bd87 100644 --- a/ui/src/components/IssueChatThread.tsx +++ b/ui/src/components/IssueChatThread.tsx @@ -60,6 +60,8 @@ import { copyTextToClipboard } from "../lib/clipboard"; import { loadDraft, loadDraftIfAvailable, + loadDraftRecoveryKey, + preserveDraftInTab, saveDraft, clearDraft, loadDraftAttachments, @@ -4653,7 +4655,7 @@ const IssueChatComposer = forwardRef< stopScope = "leaf", onImageUpload, onAttachImage, - draftKey, + draftKey: sharedDraftKey, enableReassign = false, reassignOptions = [], currentAssigneeValue = "", @@ -4673,6 +4675,14 @@ const IssueChatComposer = forwardRef< forwardedRef, ) { const stopControl = useComposerStop(onStop, stopPending); + const restoredRecovery = useMemo(() => { + const key = sharedDraftKey && loadDraftRecoveryKey(sharedDraftKey); + return key ? { sourceKey: sharedDraftKey!, key, persisted: true } : null; + }, [sharedDraftKey]); + const [newRecovery, setDraftRecovery] = useState<{ sourceKey: string; key: string; persisted: boolean } | null>(null); + const draftRecovery = newRecovery?.sourceKey === sharedDraftKey ? newRecovery : restoredRecovery; + const draftKey = draftRecovery?.key ?? sharedDraftKey; + const retiredDraftKeyRef = useRef(undefined); // Initialize before StrictMode's mount cleanup can flush an empty value over // the stored draft. The effect below handles subsequent task-key changes. const [body, setBody] = useState(() => (draftKey ? loadDraft(draftKey) : "")); @@ -4808,7 +4818,7 @@ const IssueChatComposer = forwardRef< } useEffect(() => { - if (!draftKey) return; + if (!draftKey || (draftRecovery?.key === draftKey && !draftRecovery.persisted)) return; setBody(loadDraft(draftKey)); setComposerAttachments( loadDraftAttachments(draftKey).map((item) => ({ @@ -4827,47 +4837,51 @@ const IssueChatComposer = forwardRef< const { attemptId } = uncertainSubmission; const reconciled = reconciledSubmissionRef.current; if (reconciled && reconciled.draftKey === draftKey && reconciled.attemptId === attemptId) return; - let nextDraft = uncertainSubmission.nextDraftOffset === undefined + const nextDraft = uncertainSubmission.nextDraftOffset === undefined ? "" : bodyRef.current.slice(uncertainSubmission.nextDraftOffset); - let storedAttachments: ComposerAttachmentItem[] | null = null; + const submittedIds = uncertainSubmission.submittedAttachmentIds; + let nextAttachments = submittedIds + ? composerAttachmentsRef.current.filter(item => !item.attachmentId || !submittedIds.includes(item.attachmentId)) + : []; if (draftKey) { const retained = loadDraftSubmission(draftKey); - if (retained && retained.attemptId !== attemptId) { - // Another attempt owns storage. Adopt its snapshot without settling it. - setUncertainSubmission(retained); - const storedDraft = loadDraft(draftKey); - setBody(storedDraft); - bodyRef.current = storedDraft; - setComposerAttachments(loadDraftAttachments(draftKey).map(item => ({ - ...item, size: item.size ?? 0, id: `receipt:${item.attachmentId}`, status: "attached", - }))); - return; - } - if (!settleDraftSubmission(draftKey, attemptId, nextDraft)) { - const storedDraft = loadDraftIfAvailable(draftKey); - if (storedDraft !== null && storedDraft !== bodyRef.current) { - // Another tab already settled this receipt. Its newer draft wins. - nextDraft = storedDraft; - storedAttachments = loadDraftAttachments(draftKey).map(item => ({ - ...item, size: item.size ?? 0, id: `receipt:${item.attachmentId}`, status: "attached", - })); + // Settle storage from its own snapshot, never from this tab's stale copy. + // A different retained attempt is owned by its original tab. + if (retained?.attemptId === attemptId) settleDraftSubmission(draftKey, attemptId); + const storedDraft = loadDraftIfAvailable(draftKey); + const storedAttachments = loadDraftAttachments(draftKey).filter(item => !submittedIds?.includes(item.attachmentId)); + const foreignAttempt = retained && retained.attemptId !== attemptId; + const differentAttachments = JSON.stringify(nextAttachments.map(item => item.attachmentId).sort()) !== + JSON.stringify(storedAttachments.map(item => item.attachmentId).sort()); + if (storedDraft !== null && (foreignAttempt || !retained || storedDraft !== nextDraft || differentAttachments)) { + // Text has no cross-tab ordering. Preserve each buffer separately. When + // both snapshots describe the same text, retain all attachment receipts. + if (!foreignAttempt && (storedDraft === nextDraft || storedDraft === bodyRef.current)) { + const ids = new Set(nextAttachments.map(item => item.attachmentId)); + const combined = [...nextAttachments, ...storedAttachments.filter(item => !ids.has(item.attachmentId)).map(item => ({ + ...item, size: item.size ?? 0, id: `receipt:${item.attachmentId}`, status: "attached" as const, + }))]; + // Each draft is limited to 20 receipts. If their union exceeds that, + // keep the local selection here and the other selection in shared storage. + if (combined.length <= 20) nextAttachments = combined; } - // With unavailable storage, the confirmed in-memory attempt still settles. + const recovered = preserveDraftInTab(sharedDraftKey!, nextDraft, nextAttachments); + // Fence old effect cleanups before switching keys or updating bodyRef. + retiredDraftKeyRef.current = draftKey; + setDraftRecovery({ sourceKey: sharedDraftKey!, ...recovered }); } + // With unavailable storage, the confirmed in-memory attempt still settles. } reconciledSubmissionRef.current = { draftKey, attemptId }; setUncertainSubmission(null); setBody(nextDraft); bodyRef.current = nextDraft; - const submittedIds = uncertainSubmission.submittedAttachmentIds; - setComposerAttachments(current => storedAttachments ?? (submittedIds - ? current.filter(item => !item.attachmentId || !submittedIds.includes(item.attachmentId)) - : [])); - }, [confirmedSubmissionIds, draftKey, uncertainSubmission]); + setComposerAttachments(nextAttachments); + }, [confirmedSubmissionIds, draftKey, sharedDraftKey, uncertainSubmission]); useEffect(() => { if ( - !draftKey || + !draftKey || retiredDraftKeyRef.current === draftKey || submitting || composerAttachments !== composerAttachmentsRef.current ) @@ -4884,14 +4898,14 @@ const IssueChatComposer = forwardRef< if (!draftKey || submitting) return; if (draftTimer.current) clearTimeout(draftTimer.current); draftTimer.current = setTimeout(() => { - saveDraft(draftKey, body); + if (retiredDraftKeyRef.current !== draftKey) saveDraft(draftKey, body); }, DRAFT_DEBOUNCE_MS); }, [body, draftKey, submitting]); useEffect(() => { return () => { if (draftTimer.current) clearTimeout(draftTimer.current); - if (draftKey && !submittingRef.current) + if (draftKey && retiredDraftKeyRef.current !== draftKey && !submittingRef.current) saveDraft(draftKey, bodyRef.current); }; }, [draftKey]); @@ -4899,7 +4913,7 @@ const IssueChatComposer = forwardRef< useEffect(() => { if (!draftKey) return; const flushDraft = () => { - if (!submittingRef.current) saveDraft(draftKey, bodyRef.current); + if (retiredDraftKeyRef.current !== draftKey && !submittingRef.current) saveDraft(draftKey, bodyRef.current); }; window.addEventListener("beforeunload", flushDraft); return () => window.removeEventListener("beforeunload", flushDraft); @@ -5382,6 +5396,12 @@ const IssueChatComposer = forwardRef< ) : null} + {draftRecovery && draftRecovery.sourceKey === sharedDraftKey ? ( +

+ Another tab changed the saved draft. This draft is kept separately in this tab. + {!draftRecovery.persisted ? " Browser storage is unavailable; copy your text before leaving." : " It will be restored if you reload this tab."} +

+ ) : null} {uncertainSubmission ? (
{ contentPath: `/api/attachments/${id}/content`, }; beforeEach(() => localStorage.clear()); + it("persists a tab recovery without touching shared text, receipts, or pending submission", () => { + sessionStorage.clear(); + saveDraft(key, "Other tab text"); + saveDraftAttachments(key, [receipt]); + saveDraftSubmission(key, { attemptId: id, reviewed: false }); + const fork = preserveDraftInTab(key, "This tab text", [receipt]); + expect(fork.persisted).toBe(true); + expect(loadDraftRecoveryKey(key)).toBe(fork.key); + expect(loadDraft(fork.key)).toBe("This tab text"); + expect(loadDraftAttachments(fork.key)).toEqual([receipt]); + expect(loadDraftSubmission(fork.key)).toBeNull(); + expect(loadDraft(key)).toBe("Other tab text"); + expect(loadDraftAttachments(key)).toEqual([receipt]); + expect(loadDraftSubmission(key)?.attemptId).toBe(id); + clearDraft(fork.key); + expect(loadDraft(fork.key)).toBe(""); + expect(loadDraft(key)).toBe("Other tab text"); + expect(loadDraftSubmission(key)?.attemptId).toBe(id); + }); + it("settles only submitted text and attachments while preserving the next draft", () => { const nextId = "aaf8228f-0be7-45ae-a104-6fbe0af6f1d3"; const nextReceipt = { ...receipt, attachmentId: nextId, contentPath: `/api/attachments/${nextId}/content` }; diff --git a/ui/src/lib/composer-draft.ts b/ui/src/lib/composer-draft.ts index 66b97a39ab1..35c82b56ce5 100644 --- a/ui/src/lib/composer-draft.ts +++ b/ui/src/lib/composer-draft.ts @@ -30,6 +30,27 @@ export function loadDraftIfAvailable(draftKey: string): string | null { } } +/** A conflicting tab keeps its own recoverable buffer without replacing the + * shared draft or acquiring another tab's pending receipt. */ +export function loadDraftRecoveryKey(draftKey: string): string | null { + const key = `paperclip:agent-chat-draft:recovered:${draftKey}`; + try { + return sessionStorage.getItem(`${key}:recovery:v1`) === draftKey ? key : null; + } catch { return null; } +} + +export function preserveDraftInTab(draftKey: string, body: string, attachments: unknown): { key: string; persisted: boolean } { + const key = `paperclip:agent-chat-draft:recovered:${draftKey}`; + try { + sessionStorage.setItem(key, body); + sessionStorage.setItem(`${key}:attachments:v1`, JSON.stringify({ version: 1, draftKey: key, attachments: draftAttachments(attachments) })); + sessionStorage.setItem(`${key}:recovery:v1`, draftKey); + return { key, persisted: true }; + } catch { + return { key, persisted: false }; + } +} + function mayWriteDraft(draftKey: string, attemptId?: string) { const pending = loadDraftSubmission(draftKey); return !pending || pending.attemptId === attemptId;