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; + }; }, }; }