From 597f74c2f2ca62da6376c8f4bf4b3852a107cdc9 Mon Sep 17 00:00:00 2001 From: Julius Marminge <51714798+juliusmarminge@users.noreply.github.com> Date: Sun, 25 Oct 2026 16:12:41 -0700 Subject: [PATCH] fix(web): a stale queued-edit save no longer clears a newer edit #24972 moved queued-message edits into a shared store so they survive navigation, but the save lock stayed in the chat view. Repro: 1. Start a slow save of a queued edit. 2. Navigate away from the thread. 3. Come back and edit the same message again. 4. The first save finishes, clears the newer edit's draft and ends it. The remounted view had a fresh lock, and the finishing save only knew the thread key, not which edit it belonged to. The queued-run edit store now owns saves. startSave claims the open edit (one save at a time, exposed through savingAtom for isSaving) and returns a finisher bound to that edit's identity. When the save settles after its edit was ended or begun again, the finisher changes nothing and returns false, so the hook leaves the newer edit and its draft alone. This replaces suspendExitWatch/resumeExitWatch; the rest of the design is unchanged. Co-Authored-By: Claude Opus 5.5 --- .../chat/useQueuedRunEdit.browser.test.tsx | 56 +++++++++++++++---- .../src/components/chat/useQueuedRunEdit.ts | 23 +++----- .../client-runtime/src/queuedRunEdit.test.ts | 45 +++++++++++++-- packages/client-runtime/src/queuedRunEdit.ts | 55 ++++++++++++++---- 4 files changed, 139 insertions(+), 40 deletions(-) diff --git a/apps/web/src/components/chat/useQueuedRunEdit.browser.test.tsx b/apps/web/src/components/chat/useQueuedRunEdit.browser.test.tsx index c01fa57b4e2a5..08e919cb152ec 100644 --- a/apps/web/src/components/chat/useQueuedRunEdit.browser.test.tsx +++ b/apps/web/src/components/chat/useQueuedRunEdit.browser.test.tsx @@ -126,6 +126,16 @@ async function beginChangedEdit(hook: { current: HookResult | null }) { ); } +const savedContent = (prompt: string) => ({ + prompt, + images: [], + files: [], + terminalContexts: [], + reviewComments: [], + previewAnnotations: [], + threadContexts: [], +}); + const promptOf = (ref: ScopedThreadRef) => useComposerDraftStore.getState().getComposerDraft(ref)?.prompt ?? ""; @@ -169,18 +179,7 @@ describe("useQueuedRunEdit", () => { await beginChangedEdit(hook); // Not inside act(): act would hold the projection update until the save settles. - const saved = requireHook(hook).saveEdit( - { - prompt: "changed", - images: [], - files: [], - terminalContexts: [], - reviewComments: [], - previewAnnotations: [], - threadContexts: [], - }, - () => {}, - ); + const saved = requireHook(hook).saveEdit(savedContent("changed"), () => {}); await vi.waitFor(() => expect(mocks.editQueuedRun).toHaveBeenCalledOnce()); // The server applied the edit and then started the run; the projection // reports the start before the edit's acknowledgement arrives. @@ -192,4 +191,37 @@ describe("useQueuedRunEdit", () => { expect(requireHook(hook).isEditing).toBe(false); expect(promptOf(THREAD_A)).toBe(""); }); + + it("leaves a newer edit alone when a save from before navigation finishes", async () => { + const { hook } = setup(); + let finishSave: (result: { readonly _tag: "Success" }) => void = () => {}; + mocks.editQueuedRun.mockReturnValue( + new Promise((resolve) => { + finishSave = resolve; + }), + ); + await using screen = await render(view(THREAD_A, "queued", hook)); + await beginChangedEdit(hook); + // Not inside act(): act would hold the rerenders until the save settles. + const saved = requireHook(hook).saveEdit(savedContent("changed"), () => {}); + await vi.waitFor(() => expect(mocks.editQueuedRun).toHaveBeenCalledOnce()); + + // Leaving the thread unmounts the chat view; coming back mounts a new one. + await screen.rerender({null}); + await screen.rerender(view(THREAD_A, "queued", hook)); + expect(requireHook(hook).isSaving).toBe(true); + await beginChangedEdit(hook); + await act(() => + useComposerDraftStore.getState().setPrompt(requireHook(hook).draftTarget, "newer"), + ); + + finishSave({ _tag: "Success" }); + await act(() => saved); + + expect(requireHook(hook).editingRunId).toBe(RUN); + expect(requireHook(hook).isSaving).toBe(false); + expect( + useComposerDraftStore.getState().getComposerDraft(requireHook(hook).draftTarget)?.prompt, + ).toBe("newer"); + }); }); diff --git a/apps/web/src/components/chat/useQueuedRunEdit.ts b/apps/web/src/components/chat/useQueuedRunEdit.ts index a9fbf7408dd0c..a57b1f687b7a6 100644 --- a/apps/web/src/components/chat/useQueuedRunEdit.ts +++ b/apps/web/src/components/chat/useQueuedRunEdit.ts @@ -19,7 +19,7 @@ import { import { scopedThreadKey } from "@t3tools/client-runtime/environment"; import { ATTACHMENT_ONLY_BOOTSTRAP_PROMPT } from "@t3tools/shared/composerContextReferences"; import { Atom } from "effect/reactivity"; -import { useRef, useState, type RefObject } from "react"; +import type { RefObject } from "react"; import { useAssetUrls } from "../../assets/assetUrls"; import { DraftId, @@ -193,8 +193,7 @@ export function useQueuedRunEdit(input: { const editKey = scopedThreadKey(routeThreadRef); const storedEdit = useAtomValue(queuedRunEdits.atom)[editKey] ?? null; const editing = activeThreadId === null ? null : storedEdit; - const saveInFlightRef = useRef(false); - const [isSaving, setIsSaving] = useState(false); + const isSaving = useAtomValue(queuedRunEdits.savingAtom)[editKey] === true; const draftTarget = editing === null ? threadDraftTarget : queuedEditDraftTarget(routeThreadRef, editing.runId); @@ -299,7 +298,7 @@ export function useQueuedRunEdit(input: { content: QueuedRunEditContent, setThreadError: (threadId: ThreadId, error: string | null) => void, ) => { - if (editing === null || saveInFlightRef.current) return; + if (editing === null || isSaving) return; const newAttachments = [...content.images, ...content.files]; if ( editing.existingAttachments.length + newAttachments.length > @@ -318,10 +317,9 @@ export function useQueuedRunEdit(input: { ) { return; } - saveInFlightRef.current = true; - setIsSaving(true); // The save's result decides the edit's fate, even if the run starts meanwhile. - queuedRunEdits.suspendExitWatch(editKey); + const finishSave = queuedRunEdits.startSave(editKey); + if (finishSave === null) return; const error = await submitQueuedRunEdit({ environmentId, draftTarget, @@ -329,18 +327,15 @@ export function useQueuedRunEdit(input: { content, editQueuedRun: editQueuedRunCommand, }); - saveInFlightRef.current = false; - setIsSaving(false); setThreadError(editing.threadId, error); - if (error !== null) { - queuedRunEdits.resumeExitWatch(editKey); - return; - } + const current = finishSave(error === null); + if (error !== null) return; releaseDraftAttachments(newAttachments); + // The edit was cancelled or begun again while this save ran; its drafts are the newer edit's. + if (!current) return; promptRef.current = ""; clearEditDraft(editing.runId); composerRef.current?.resetCursorState(); - queuedRunEdits.end(editKey); focusComposerSoon(); }; diff --git a/packages/client-runtime/src/queuedRunEdit.test.ts b/packages/client-runtime/src/queuedRunEdit.test.ts index b5f560f849f50..b9d22b3fa9a51 100644 --- a/packages/client-runtime/src/queuedRunEdit.test.ts +++ b/packages/client-runtime/src/queuedRunEdit.test.ts @@ -78,13 +78,50 @@ describe("makeQueuedRunEditStore", () => { it("holds the exit during a save and replays it when the save fails", () => { const { registry, runs, store, exits, begin } = setup(); begin(); - store.suspendExitWatch("env:thread"); + const finishSave = store.startSave("env:thread"); registry.set(runs, []); expect(exits).toEqual([]); - store.resumeExitWatch("env:thread"); + expect(finishSave?.(false)).toBe(true); expect(exits).toHaveLength(1); }); + it("allows one save at a time and ends the edit when it succeeds", () => { + const { registry, store, begin } = setup(); + begin(); + const finishSave = store.startSave("env:thread"); + expect(registry.get(store.savingAtom)).toEqual({ "env:thread": true }); + expect(store.startSave("env:thread")).toBeNull(); + expect(finishSave?.(true)).toBe(true); + expect(store.get("env:thread")).toBeNull(); + expect(registry.get(store.savingAtom)).toEqual({}); + }); + + it("leaves an edit begun again during a save to that edit", () => { + const { registry, runs, store, exits, begin } = setup(); + begin(); + const finishStale = store.startSave("env:thread"); + begin(); + store.update("env:thread", (edit) => ({ ...edit, text: "newer" })); + expect(registry.get(store.savingAtom)).toEqual({}); + + expect(finishStale?.(true)).toBe(false); + expect(finishStale?.(false)).toBe(false); + expect(store.get("env:thread")).toEqual({ runId: RUN, text: "newer" }); + registry.set(runs, []); + expect(exits).toEqual([{ threadKey: "env:thread", text: "newer" }]); + }); + + it("does not revive a cancelled edit when its save fails", () => { + const { registry, runs, store, exits, begin } = setup(); + begin(); + const finishSave = store.startSave("env:thread"); + store.end("env:thread"); + expect(finishSave?.(false)).toBe(false); + registry.set(runs, []); + expect(exits).toEqual([]); + expect(registry.get(store.savingAtom)).toEqual({}); + }); + // The runs read the thread's detail, a live server subscription; a thread // with no edit open must not keep it. it("holds the thread's runs only while an edit is open and watching", () => { @@ -122,10 +159,10 @@ describe("makeQueuedRunEditStore", () => { expect(mounted).toBe(0); begin(); - store.suspendExitWatch("env:thread"); + const finishSave = store.startSave("env:thread"); settle(); expect(mounted).toBe(0); - store.resumeExitWatch("env:thread"); + finishSave?.(false); expect(mounted).toBe(1); registry.set(source, []); diff --git a/packages/client-runtime/src/queuedRunEdit.ts b/packages/client-runtime/src/queuedRunEdit.ts index 809e8dcfa6c97..65b29647c934b 100644 --- a/packages/client-runtime/src/queuedRunEdit.ts +++ b/packages/client-runtime/src/queuedRunEdit.ts @@ -1,7 +1,8 @@ /** * A queued-message edit ends on save, cancel, or when its run leaves the queue. * The store owns one watcher per edit, so "run left the queue" is handled exactly - * once, when the projection reports it, wherever the user is in the app. + * once, when the projection reports it, wherever the user is in the app. It also + * owns the edit's save, because a save outlives the view that started it. */ import type { RunId } from "@t3tools/contracts"; import { Atom, type AtomRegistry } from "effect/reactivity"; @@ -67,6 +68,14 @@ interface ExitWatch { cancel: (() => void) | null; } +/** + * Settles a save started with `startSave`. Returns false, changing nothing, + * when the edit the save started from was ended or begun again meanwhile; the + * caller must then leave the edit's drafts alone. Otherwise success ends the + * edit and failure resumes its exit watch. + */ +export type FinishQueuedRunEditSave = (succeeded: boolean) => boolean; + export function makeQueuedRunEditStore(options: { readonly registry: AtomRegistry.AtomRegistry; readonly label: string; @@ -76,8 +85,22 @@ export function makeQueuedRunEditStore(opti Atom.keepAlive, Atom.withLabel(options.label), ); + const savingAtom = Atom.make>>({}).pipe( + Atom.keepAlive, + Atom.withLabel(`${options.label}-saving`), + ); + // Each begin creates a new watch, so the watch is the edit's identity. const watches = new Map>(); + const setSaving = (threadKey: string, saving: boolean) => { + const current = registry.get(savingAtom); + if (threadKey in current === saving) return; + const next = { ...current }; + if (saving) next[threadKey] = true; + else delete next[threadKey]; + registry.set(savingAtom, next); + }; + const get = (threadKey: string | null): E | null => threadKey === null ? null : (registry.get(atom)[threadKey] ?? null); @@ -118,10 +141,13 @@ export function makeQueuedRunEditStore(opti if (watch === undefined) return; unsubscribe(watch); watches.delete(threadKey); + setSaving(threadKey, false); }; return { atom, + /** Thread keys whose edit has a save in flight. */ + savingAtom, get, /** * Starts an edit, replacing any edit already open on the thread. When the @@ -151,17 +177,26 @@ export function makeQueuedRunEditStore(opti write(threadKey, null); }, /** - * Holds the exit transition while a save is in flight; the save's own - * result decides the edit's fate. + * Claims the open edit for a save, or returns null when there is no edit + * or a save already holds it. The exit watch pauses so the save's result + * decides the edit's fate. */ - suspendExitWatch(threadKey: string) { + startSave(threadKey: string): FinishQueuedRunEditSave | null { const watch = watches.get(threadKey); - if (watch !== undefined) unsubscribe(watch); - }, - /** Resumes after a failed save, handling an exit that happened meanwhile. */ - resumeExitWatch(threadKey: string) { - const watch = watches.get(threadKey); - if (watch !== undefined && watch.cancel === null) subscribe(threadKey, watch); + if (watch === undefined || threadKey in registry.get(savingAtom)) return null; + setSaving(threadKey, true); + unsubscribe(watch); + return (succeeded) => { + if (watches.get(threadKey) !== watch) return false; + if (succeeded) { + stopWatch(threadKey); + write(threadKey, null); + } else { + setSaving(threadKey, false); + subscribe(threadKey, watch); + } + return true; + }; }, }; }