mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-02 02:07:25 +08:00
fix(ui): preserve newer drafts after repeated receipt cleanup (#14332)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The task composer keeps unsent text across page reloads. > - A server receipt confirms a submitted message after a reload. > - Replayed cleanup can apply the old text offset to a newer draft twice. > - This pull request reconciles each confirmed attempt once and preserves both tabs’ unsent intent. > - The full newer draft stays available for the next send. ## Linked Issues or Issue Description **What happened?** Reloading the classic composer while a save was pending could truncate a newer draft. Replayed receipt cleanup changed `A newer draft written while delivery was pending.` into `nding.`. The storage helper rejected the duplicate settlement, but the effect still changed the editor and its body reference. **Expected behavior** A receipt settles its retained submission once. Later cleanup must preserve the newer draft in both the editor and browser storage. **Steps to reproduce** 1. Send a comment and hold its HTTP response after the server accepts it. 2. Type a newer draft, then reload the page. 3. Restore the matching receipt under React StrictMode. 4. Inspect the newer draft after effect replay and unmount. The deterministic regression reproduces this on current master. The existing browser test exposed the issue during #14329 verification. Related draft persistence code came from #13338. A search found no separate fix for this duplicate settlement. ## What Changed - Reconcile each confirmed attempt once in memory so duplicate effects cannot trim its newer draft again. - Unlock confirmed drafts when storage writes fail. Never acquire another tab’s pending receipt or assume its text is newer. Keep a conflicting local draft and its attachments in tab-scoped session storage while preserving the shared draft unchanged. - Add component regressions for StrictMode replay, exact editor and stored bytes, foreign receipts, attachment-only differences, reload recovery, unavailable storage, and task navigation before autosave. A brief notice explains when this tab has a separate draft. ## Verification - RED: the new test received `nding.` instead of the full newer draft before the fix. - Review RED: three cases reproduced a locked composer after failed storage writes or another tab's settlement. A further negative case preserves a newer retained attempt and its attachments. - Cross-tab review RED: four deterministic cases reproduced lost local text, foreign receipt takeover, attachment loss when text matched, and overwriting newer stored text. - GREEN: 118 tests across `IssueChatThread`, `composer-draft`, and `comment-submit-draft`, including same-mounted A → B → A navigation and a full recovery-storage failure. Two further lifecycle RED tests verify finishing a recovery returns to the shared draft on a later visit while continued typing and attachments retain recovery. - Both existing browser reload cases passed against a fresh server and database on exact head `ceb80aca77fc8cc0f813c328ba87025b3e1a2222` (42.5 seconds), with classic mode enabled and disabled. The browser flow verifies one original comment, a preserved draft, and a successful second send. - UI typecheck, token gates, and the shipped static UI build passed. The initial typecheck required the fresh worktree's plugin SDK build; the retry passed after that dependency built. - The session-only failure mock also preserves localStorage on platforms where both share the Storage prototype; this fixes the Linux workspace test failure. - All 56 checks passed on `4c30dcccc4fe5b90dcd08dd1d90475ea89e4a376`; Greptile is 5/5 with zero open review threads. An unrelated runner baseline scan hit its existing 100 ms deadline once; its isolated test and the single failed CI job passed on retry without source changes. This four-file UI fix does not change server or database code. - Final alternate-staging acceptance passed on deployed source `d884e1ab046cc76004e35e6091e9e6e2c918c9eb`, with exact health checked before and after. Three real browser cases used shipped static UI and real API saves: reload during an accepted-but-unacknowledged save in both composer variants, plus two classic tabs with different drafts. The test deliberately held only its own accepted POST acknowledgement and temporarily withheld its own GET receipt from one tab to reproduce settlement ordering. Both drafts survived independent reloads, the shared draft remained intact after the other tab sent, and finishing recovery returned to the shared draft on a later visit. Exact request IDs/counts and server comment bytes passed; no provider was invoked. Screenshots preserve the live drafts before fixture cleanup. - An ordinary authenticated Chrome/CUA visit independently showed the exact original and newer comments; an additional unsent draft survived reload and was then cleared. All three isolated fixtures are complete, browser contexts are closed, and original user tasks were untouched. ## Risks - Reconciliation uses the exact request ID and draft key. Conflicting drafts stay separate; the tab recovery survives same-tab reload through session storage and does not outlive the browser tab. If recovery storage is unavailable, the editor keeps its in-memory text and explicitly warns the user to copy it before leaving. - Attachment selections remain bounded to 20 per draft; if combining equal-text snapshots would exceed that limit, each original selection stays in its respective draft. - No API, schema, or migration changes. The status notice uses existing design tokens. ## Model Used OpenAI Codex, GPT-6, with tool use and code execution. The runtime does not expose the exact deployment variant or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -15,6 +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 { loadDraft, preserveDraftInTab, settleDraftSubmission } from "../lib/composer-draft";
|
||||
import {
|
||||
IssueAssigneePausedNotice,
|
||||
IssueChatThread,
|
||||
@@ -379,6 +380,7 @@ describe("IssueChatThread", () => {
|
||||
document.body.appendChild(container);
|
||||
window.scrollTo = vi.fn();
|
||||
localStorage.clear();
|
||||
sessionStorage.clear();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
@@ -3276,6 +3278,234 @@ describe("IssueChatThread", () => {
|
||||
act(() => root.unmount());
|
||||
});
|
||||
|
||||
it("settles a restored submission only once through StrictMode effect replay", () => {
|
||||
const key = "strict-restored-submission";
|
||||
const attemptId = "aaf8228f-0be7-45ae-a104-6fbe0af6f1d3";
|
||||
const submitted = "One text-only save interrupted by reload.";
|
||||
const nextDraft = "A newer draft written while delivery was pending.";
|
||||
localStorage.setItem(key, `${submitted}\n\n${nextDraft}`);
|
||||
localStorage.setItem(`${key}:submission:v1`, JSON.stringify({
|
||||
version: 1,
|
||||
draftKey: key,
|
||||
attemptId,
|
||||
reviewed: false,
|
||||
nextDraftOffset: submitted.length + 2,
|
||||
submittedAttachmentIds: [],
|
||||
}));
|
||||
const root = createRoot(container);
|
||||
try {
|
||||
act(() => root.render(
|
||||
<StrictMode>
|
||||
<MemoryRouter>
|
||||
<IssueChatThread
|
||||
comments={[{
|
||||
...issueChatLongThreadComments[0]!,
|
||||
id: "confirmed-restored-comment",
|
||||
body: submitted,
|
||||
authorAgentId: null,
|
||||
authorUserId: "user-1",
|
||||
clientRequestId: attemptId,
|
||||
}]}
|
||||
currentUserId="user-1"
|
||||
linkedRuns={[]}
|
||||
timelineEvents={[]}
|
||||
liveRuns={[]}
|
||||
onAdd={async () => {}}
|
||||
draftKey={key}
|
||||
enableLiveTranscriptPolling={false}
|
||||
/>
|
||||
</MemoryRouter>
|
||||
</StrictMode>,
|
||||
));
|
||||
expect(container.querySelector<HTMLTextAreaElement>(
|
||||
'textarea[aria-label="Issue chat editor"]',
|
||||
)?.value).toBe(nextDraft);
|
||||
expect(localStorage.getItem(key)).toBe(nextDraft);
|
||||
expect(localStorage.getItem(`${key}:submission:v1`)).toBeNull();
|
||||
expect(container.textContent).not.toContain("We couldn’t confirm");
|
||||
} finally {
|
||||
act(() => root.unmount());
|
||||
}
|
||||
expect(localStorage.getItem(key)).toBe(nextDraft);
|
||||
});
|
||||
|
||||
it.each(["submission write", "all storage"])(
|
||||
"unlocks a confirmed in-memory submission after failed %s",
|
||||
async (failure) => {
|
||||
const key = "unavailable-submission-storage";
|
||||
const originalSetItem = localStorage.setItem.bind(localStorage);
|
||||
const write = vi.spyOn(localStorage, "setItem").mockImplementation((key, value) => {
|
||||
if (failure === "all storage" || key.endsWith(":submission:v1")) throw new Error("Storage unavailable");
|
||||
originalSetItem(key, value);
|
||||
});
|
||||
const read = failure === "all storage"
|
||||
? vi.spyOn(localStorage, "getItem").mockImplementation(() => { throw new Error("Storage unavailable"); })
|
||||
: null;
|
||||
let rejectSend!: (error: Error) => void;
|
||||
const onAdd = vi.fn().mockReturnValueOnce(new Promise<void>((_, reject) => { rejectSend = reject; })).mockResolvedValue(undefined);
|
||||
const root = createRoot(container);
|
||||
const element = (attemptId?: string) => (
|
||||
<MemoryRouter>
|
||||
<IssueChatThread
|
||||
comments={attemptId ? [{ ...issueChatLongThreadComments[0]!, id: "confirmed-memory-comment", body: "Earlier message", authorAgentId: null, authorUserId: "user-1", clientRequestId: attemptId }] : []}
|
||||
currentUserId="user-1" linkedRuns={[]} timelineEvents={[]} liveRuns={[]}
|
||||
onAdd={onAdd} draftKey={key} enableLiveTranscriptPolling={false}
|
||||
/>
|
||||
</MemoryRouter>
|
||||
);
|
||||
const editor = () => container.querySelector<HTMLTextAreaElement>('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 }));
|
||||
});
|
||||
const send = () => Array.from(container.querySelectorAll("button")).find(button => button.textContent === "Send") as HTMLButtonElement;
|
||||
try {
|
||||
await act(async () => root.render(element()));
|
||||
type("Earlier message");
|
||||
await act(async () => send().click());
|
||||
const attemptId = onAdd.mock.calls[0]![4] as string;
|
||||
type("The full next draft");
|
||||
await act(async () => rejectSend(new CommentSubmissionUnknownError()));
|
||||
expect(container.textContent).toContain("We couldn’t confirm");
|
||||
await act(async () => root.render(element(attemptId)));
|
||||
expect(editor().value).toBe("The full next draft");
|
||||
expect(container.textContent).not.toContain("We couldn’t confirm");
|
||||
expect(send().disabled).toBe(false);
|
||||
await act(async () => send().click());
|
||||
expect(onAdd.mock.calls[1]![0]).toBe("The full next draft");
|
||||
} finally {
|
||||
await act(async () => root.unmount());
|
||||
write.mockRestore();
|
||||
read?.mockRestore();
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
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";
|
||||
const attachmentId = "caf8228f-0be7-45ae-a104-6fbe0af6f1d3";
|
||||
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) => (
|
||||
<MemoryRouter>
|
||||
<IssueChatThread
|
||||
comments={confirmed ? [
|
||||
{ ...issueChatLongThreadComments[0]!, id: "confirmed-cross-tab-comment", body: "Earlier message", authorAgentId: null, authorUserId: "user-1", clientRequestId: attemptId },
|
||||
...(foreignConfirmed ? [{ ...issueChatLongThreadComments[0]!, id: "other-tab-confirmed", body: "Other tab message", authorAgentId: null, authorUserId: "user-1", clientRequestId: newerId }] : []),
|
||||
] : []}
|
||||
currentUserId="user-1" linkedRuns={[]} timelineEvents={[]} liveRuns={[]}
|
||||
onAdd={async () => {}} draftKey={key} enableLiveTranscriptPolling={false}
|
||||
/>
|
||||
</MemoryRouter>
|
||||
);
|
||||
let root = createRoot(container);
|
||||
const storagePrototype = Object.getPrototypeOf(sessionStorage) as Storage;
|
||||
const originalStorageWrite = storagePrototype.setItem;
|
||||
const recoveryWrite = scenario === "unavailable recovery storage"
|
||||
? vi.spyOn(storagePrototype, "setItem").mockImplementation(function (this: Storage, key: string, value: string) {
|
||||
if (this === sessionStorage) throw new Error("Storage full");
|
||||
return originalStorageWrite.call(this, key, value);
|
||||
})
|
||||
: 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<HTMLTextAreaElement>('textarea[aria-label="Issue chat editor"]')!;
|
||||
expect(editor().value).toBe(localText);
|
||||
expect(container.textContent).not.toContain("We couldn’t confirm");
|
||||
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();
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
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) => (
|
||||
<MemoryRouter><IssueChatThread comments={[]} linkedRuns={[]} timelineEvents={[]} liveRuns={[]}
|
||||
onAdd={async () => {}} draftKey={draftKey} enableLiveTranscriptPolling={false} /></MemoryRouter>
|
||||
);
|
||||
const editor = () => container.querySelector<HTMLTextAreaElement>('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("returns to the shared draft after finishing a recovery and leaving the task", async () => {
|
||||
const a = "finished-recovery-a";
|
||||
const b = "finished-recovery-b";
|
||||
localStorage.setItem(a, "The other tab's preserved draft");
|
||||
preserveDraftInTab(a, "My recovered message", []);
|
||||
const onAdd = vi.fn().mockResolvedValue(undefined);
|
||||
const root = createRoot(container);
|
||||
const element = (draftKey: string) => (
|
||||
<MemoryRouter><IssueChatThread comments={[]} linkedRuns={[]} timelineEvents={[]} liveRuns={[]}
|
||||
onAdd={onAdd} draftKey={draftKey} enableLiveTranscriptPolling={false} /></MemoryRouter>
|
||||
);
|
||||
const editor = () => container.querySelector<HTMLTextAreaElement>('textarea[aria-label="Issue chat editor"]')!;
|
||||
try {
|
||||
await act(async () => root.render(element(a)));
|
||||
expect(editor().value).toBe("My recovered message");
|
||||
await act(async () => (Array.from(container.querySelectorAll("button")).find(button => button.textContent === "Send") as HTMLButtonElement).click());
|
||||
expect(onAdd.mock.calls[0]![0]).toBe("My recovered message");
|
||||
expect(editor().value).toBe("");
|
||||
await act(async () => root.render(element(b)));
|
||||
await act(async () => root.render(element(a)));
|
||||
expect(editor().value).toBe("The other tab's preserved draft");
|
||||
expect(container.textContent).not.toContain("This draft is kept separately");
|
||||
} finally { await act(async () => root.unmount()); }
|
||||
});
|
||||
|
||||
it("stores and restores the composer draft per issue key", () => {
|
||||
vi.useFakeTimers();
|
||||
const root = createRoot(container);
|
||||
|
||||
@@ -59,6 +59,9 @@ import { useOptionalToastActions } from "../context/ToastContext";
|
||||
import { copyTextToClipboard } from "../lib/clipboard";
|
||||
import {
|
||||
loadDraft,
|
||||
loadDraftIfAvailable,
|
||||
loadDraftRecoveryKey,
|
||||
preserveDraftInTab,
|
||||
saveDraft,
|
||||
clearDraft,
|
||||
loadDraftAttachments,
|
||||
@@ -4652,7 +4655,7 @@ const IssueChatComposer = forwardRef<
|
||||
stopScope = "leaf",
|
||||
onImageUpload,
|
||||
onAttachImage,
|
||||
draftKey,
|
||||
draftKey: sharedDraftKey,
|
||||
enableReassign = false,
|
||||
reassignOptions = [],
|
||||
currentAssigneeValue = "",
|
||||
@@ -4672,6 +4675,17 @@ 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;
|
||||
// Keep the active buffer through send completion. Re-entering a task may
|
||||
// return to its shared draft after an empty recovery has been retired.
|
||||
useEffect(() => setDraftRecovery(restoredRecovery), [sharedDraftKey, restoredRecovery]);
|
||||
const retiredDraftKeyRef = useRef<string | undefined>(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) : ""));
|
||||
@@ -4691,6 +4705,7 @@ const IssueChatComposer = forwardRef<
|
||||
}, [draftKey]);
|
||||
const bodyRef = useRef(body);
|
||||
bodyRef.current = body;
|
||||
const reconciledSubmissionRef = useRef<{ draftKey: string | undefined; attemptId: string } | null>(null);
|
||||
const pendingDraftRef = useRef<{
|
||||
draftKey: string;
|
||||
attemptId: string;
|
||||
@@ -4806,7 +4821,7 @@ const IssueChatComposer = forwardRef<
|
||||
}
|
||||
|
||||
useEffect(() => {
|
||||
if (!draftKey) return;
|
||||
if (!draftKey || (draftRecovery?.key === draftKey && !draftRecovery.persisted)) return;
|
||||
setBody(loadDraft(draftKey));
|
||||
setComposerAttachments(
|
||||
loadDraftAttachments(draftKey).map((item) => ({
|
||||
@@ -4822,21 +4837,54 @@ const IssueChatComposer = forwardRef<
|
||||
// Text equality is not delivery proof: users may intentionally repeat text.
|
||||
useEffect(() => {
|
||||
if (!uncertainSubmission || !confirmedSubmissionIds.has(uncertainSubmission.attemptId)) return;
|
||||
const { attemptId } = uncertainSubmission;
|
||||
const reconciled = reconciledSubmissionRef.current;
|
||||
if (reconciled && reconciled.draftKey === draftKey && reconciled.attemptId === attemptId) return;
|
||||
const nextDraft = uncertainSubmission.nextDraftOffset === undefined
|
||||
? "" : bodyRef.current.slice(uncertainSubmission.nextDraftOffset);
|
||||
if (draftKey) settleDraftSubmission(draftKey, uncertainSubmission.attemptId, nextDraft);
|
||||
const submittedIds = uncertainSubmission.submittedAttachmentIds;
|
||||
let nextAttachments = submittedIds
|
||||
? composerAttachmentsRef.current.filter(item => !item.attachmentId || !submittedIds.includes(item.attachmentId))
|
||||
: [];
|
||||
if (draftKey) {
|
||||
const retained = loadDraftSubmission(draftKey);
|
||||
// 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;
|
||||
}
|
||||
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 => 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
|
||||
)
|
||||
@@ -4853,14 +4901,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]);
|
||||
@@ -4868,7 +4916,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);
|
||||
@@ -5351,6 +5399,12 @@ const IssueChatComposer = forwardRef<
|
||||
</div>
|
||||
) : null}
|
||||
|
||||
{draftRecovery && draftRecovery.sourceKey === sharedDraftKey ? (
|
||||
<p role="status" className="mb-3 text-sm text-muted-foreground">
|
||||
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."}
|
||||
</p>
|
||||
) : null}
|
||||
{uncertainSubmission ? (
|
||||
<div
|
||||
role="alert"
|
||||
|
||||
@@ -3,6 +3,8 @@ import { beforeEach, describe, expect, it } from "vitest";
|
||||
import {
|
||||
clearDraft,
|
||||
loadDraft,
|
||||
loadDraftRecoveryKey,
|
||||
preserveDraftInTab,
|
||||
loadDraftAttachments,
|
||||
saveDraft,
|
||||
saveDraftAttachments,
|
||||
@@ -23,6 +25,48 @@ describe("task draft upload receipts", () => {
|
||||
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("retires a finished recovery but keeps or restores its mapping for unsent work", () => {
|
||||
sessionStorage.clear();
|
||||
saveDraft(key, "Shared draft stays available");
|
||||
const fork = preserveDraftInTab(key, "Recovered message", []);
|
||||
saveDraftSubmission(fork.key, { attemptId: id, reviewed: false });
|
||||
expect(settleDraftSubmission(fork.key, id)).toBe(true);
|
||||
expect(loadDraftRecoveryKey(key)).toBeNull();
|
||||
expect(loadDraft(key)).toBe("Shared draft stays available");
|
||||
|
||||
saveDraft(fork.key, "Next recovered draft");
|
||||
expect(loadDraftRecoveryKey(key)).toBe(fork.key);
|
||||
saveDraftSubmission(fork.key, { attemptId: id, reviewed: false, nextDraftOffset: 0, submittedAttachmentIds: [] });
|
||||
expect(settleDraftSubmission(fork.key, id)).toBe(true);
|
||||
expect(loadDraftRecoveryKey(key)).toBe(fork.key);
|
||||
expect(loadDraft(fork.key)).toBe("Next recovered draft");
|
||||
clearDraft(fork.key);
|
||||
expect(loadDraftRecoveryKey(key)).toBeNull();
|
||||
saveDraftAttachments(fork.key, [receipt]);
|
||||
expect(loadDraftRecoveryKey(key)).toBe(fork.key);
|
||||
expect(loadDraftAttachments(fork.key)).toEqual([receipt]);
|
||||
});
|
||||
|
||||
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` };
|
||||
|
||||
@@ -18,10 +18,47 @@ function draftStorage(draftKey: string): Storage {
|
||||
}
|
||||
|
||||
export function loadDraft(draftKey: string): string {
|
||||
return loadDraftIfAvailable(draftKey) ?? "";
|
||||
}
|
||||
|
||||
/** Distinguish an empty stored draft from unavailable browser storage. */
|
||||
export function loadDraftIfAvailable(draftKey: string): string | null {
|
||||
try {
|
||||
return draftStorage(draftKey).getItem(draftKey) ?? "";
|
||||
} catch {
|
||||
return "";
|
||||
return 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 syncDraftRecoveryMarker(draftKey: string) {
|
||||
const prefix = "paperclip:agent-chat-draft:recovered:";
|
||||
if (!draftKey.startsWith(prefix)) return;
|
||||
const marker = `${draftKey}:recovery:v1`;
|
||||
if (loadDraft(draftKey).trim() || loadDraftAttachments(draftKey).length || loadDraftSubmission(draftKey)) {
|
||||
sessionStorage.setItem(marker, draftKey.slice(prefix.length));
|
||||
} else {
|
||||
sessionStorage.removeItem(marker);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -38,6 +75,7 @@ export function saveDraft(draftKey: string, value: string, attemptId?: string) {
|
||||
} else {
|
||||
draftStorage(draftKey).removeItem(draftKey);
|
||||
}
|
||||
syncDraftRecoveryMarker(draftKey);
|
||||
} catch {
|
||||
// Ignore browser storage failures.
|
||||
}
|
||||
@@ -49,6 +87,7 @@ export function clearDraft(draftKey: string, attemptId?: string) {
|
||||
draftStorage(draftKey).removeItem(draftKey);
|
||||
draftStorage(draftKey).removeItem(`${draftKey}:attachments:v1`);
|
||||
draftStorage(draftKey).removeItem(`${draftKey}:submission:v1`);
|
||||
syncDraftRecoveryMarker(draftKey);
|
||||
} catch {
|
||||
// Ignore browser storage failures.
|
||||
}
|
||||
@@ -101,6 +140,7 @@ export function saveDraftSubmission(
|
||||
`${draftKey}:submission:v1`,
|
||||
JSON.stringify({ version: 1, draftKey, ...submission }),
|
||||
);
|
||||
syncDraftRecoveryMarker(draftKey);
|
||||
} catch {
|
||||
/* The composer also retains the fence in memory. */
|
||||
}
|
||||
@@ -108,8 +148,10 @@ export function saveDraftSubmission(
|
||||
|
||||
export function clearDraftSubmission(draftKey: string, attemptId: string) {
|
||||
try {
|
||||
if (loadDraftSubmission(draftKey)?.attemptId === attemptId)
|
||||
if (loadDraftSubmission(draftKey)?.attemptId === attemptId) {
|
||||
draftStorage(draftKey).removeItem(`${draftKey}:submission:v1`);
|
||||
syncDraftRecoveryMarker(draftKey);
|
||||
}
|
||||
} catch {
|
||||
/* Disabled browser storage is supported in memory. */
|
||||
}
|
||||
@@ -212,6 +254,7 @@ export function saveDraftAttachments(draftKey: string, attachments: unknown, att
|
||||
JSON.stringify({ version: 1, draftKey, attachments: selected }),
|
||||
);
|
||||
else draftStorage(draftKey).removeItem(`${draftKey}:attachments:v1`);
|
||||
syncDraftRecoveryMarker(draftKey);
|
||||
} catch {
|
||||
/* Disabled/full browser storage must not break the composer. */
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user