diff --git a/src/features/messages/MessageComposer.test.tsx b/src/features/messages/MessageComposer.test.tsx index 0b0c20f6d1..d5e62daf07 100644 --- a/src/features/messages/MessageComposer.test.tsx +++ b/src/features/messages/MessageComposer.test.tsx @@ -974,6 +974,9 @@ async function mountUploadComposer( editable?: boolean; media?: (url: string) => string | undefined; extensions?: MessageComposerProps["extensions"]; + agents?: Agents; + /** Channel members besides the viewer; only members can be mentioned. */ + members?: readonly string[]; } = {}, ) { vi.stubGlobal( @@ -1021,7 +1024,14 @@ async function mountUploadComposer( ? [roster(relay, "other", other.members, other.time)] : [metadata(relay, "other", "Random", other.time)] : filter.kinds?.includes(39002) - ? [roster(relay, "channel", [viewer.pubkey], 1700000000)] + ? [ + roster( + relay, + "channel", + [viewer.pubkey, ...(options.members ?? [])], + 1700000000, + ), + ] : filter.kinds?.includes(39000) ? [metadata(relay, "channel", "General", 1700000000)] : [], @@ -1051,17 +1061,21 @@ async function mountUploadComposer( ); const scope = `https://relay.example.test:${viewer.pubkey}`; const view = render( - , + + + , { reactStrictMode: true, wrapper: ToastProvider }, ); const input = () => within(view.container).getByRole("textbox"); @@ -1211,6 +1225,128 @@ it("restores a failed background upload into the composer with Desktop's toast", ); }); +// Remounting after the failure covers the usual leave-and-return case: the +// recovery lands with no mounted listener and must wait for the composer. +it.each(["never", "before the failure", "after the failure"])( + "restores the model and effort picks of a failed background upload (remounted: %s)", + async (remounted) => { + // Snapshots are stable, as the service's are. + const snapshot = { + status: "ready", + agents: [{ pubkey: first.pubkey, name: "Honey", type: "test/agent" }], + }; + const models = { + current: "fast", + models: [ + { + id: "fast", + label: "Fast", + efforts: [ + { id: "low", label: "Low" }, + { id: "high", label: "High" }, + ], + }, + ], + }; + const types = [{ key: "test/agent", models: () => models }]; + const agents = { + subscribe: () => () => {}, + snapshot: () => snapshot, + types: () => types, + } as unknown as Agents; + const tools: readonly Contribution[] = [ + { + id: "fixture", + key: "test/fixture", + pluginId: "test", + revision: "1", + title: "Fixture tools", + component: (props: ComposerToolProps) => ( + + ), + }, + ]; + const extensions = { + tools: { snapshot: () => tools, subscribe: () => () => {} }, + inline: { snapshot: () => [], subscribe: () => () => {} }, + }; + const h = await mountUploadComposer({ + agents, + members: [first.pubkey], + extensions, + }); + const picker = () => + screen.findByRole("button", { + name: "Model and effort for the next run", + }); + attachByPaste(h.input(), attachmentFile("tuned.txt")); + await userEvent.type(h.input(), "think hard"); + await userEvent.click(screen.getByRole("button", { name: "First Honey" })); + await userEvent.click(await picker()); + const slider = await screen.findByLabelText("Effort for Honey's next run"); + slider.focus(); + await userEvent.keyboard("{ArrowRight}"); + expect(slider).toHaveAttribute("aria-valuetext", "High"); + await userEvent.keyboard("{Escape}"); + fireEvent.click(h.send()); + await waitFor(() => expect(h.uploadCalls).toHaveLength(1)); + + let container = h.container; + if (remounted !== "never") h.unmount(); + const remount = () => { + container = render( + + + , + { wrapper: ToastProvider }, + ).container; + }; + if (remounted === "before the failure") remount(); + await act(async () => { + h.uploadCalls[0]?.result.reject(new Error("offline")); + }); + if (remounted === "after the failure") remount(); + const input = () => within(container).getByRole("textbox"); + await waitFor(() => expect(input()).toHaveValue("think hard@Honey ")); + expect(within(container).getByText("tuned.txt")).toBeVisible(); + // The restored draft brings its pick back with it, not the default run. + const button = await picker(); + expect(button).toHaveAttribute("data-selected"); + await userEvent.hover(button); + expect(await screen.findByRole("tooltip")).toHaveTextContent( + "Next run: High", + ); + await userEvent.unhover(button); + expect(h.publish).not.toHaveBeenCalled(); + + await userEvent.click( + screen.getByRole("button", { name: "Retry tuned.txt" }), + ); + fireEvent.click( + within(container).getByRole("button", { name: "Send message" }), + ); + await waitFor(() => expect(h.uploadCalls).toHaveLength(2)); + await act(async () => { + h.uploadCalls[1]?.result.resolve(uploadDescriptor("tuned.txt")); + }); + await waitFor(() => expect(h.publish).toHaveBeenCalledTimes(1)); + expect(h.publish.mock.calls[0]?.[0].tags).toEqual( + expect.arrayContaining([ + ["p", first.pubkey], + ["effort", first.pubkey, "high"], + ]), + ); + }, +); + it.each(["cleanup", "restore"] as const)( "retains a failed attachment send until %s storage recovers", async (phase) => { diff --git a/src/features/messages/MessageComposer.tsx b/src/features/messages/MessageComposer.tsx index 24c204d39e..2e0722ffc0 100644 --- a/src/features/messages/MessageComposer.tsx +++ b/src/features/messages/MessageComposer.tsx @@ -116,19 +116,21 @@ type AcceptedDraft = { // Failed post-acceptance cleanup survives composer remounts within this session. // This is recovery evidence only, not another persistent draft inventory. const acceptedDrafts = new WeakMap>(); -type RecoveredSend = { draft: MentionDraft; preparationError?: unknown }; +type RecoveredSend = { + draft: MentionDraft; + picks?: MentionPicks; + preparationError?: unknown; +}; const recoveryListeners = new WeakMap< RelaySession, Map void>> >(); -// Only undelivered preparation failures need replay. The saved draft and -// attachment store remain the owners of the actual recovered content. -const missedPreparation = new WeakMap< - RelaySession, - Map ->(); -function takeMissedPreparation(session: RelaySession, key: string) { - const byKey = missedPreparation.get(session); +// A recovery that no mounted composer consumed replays on remount. Only the +// unpersisted parts need it: the pick snapshot and a preparation failure. The +// saved draft and attachment store remain the owners of the recovered content. +const missedRecovery = new WeakMap>(); +function takeMissedRecovery(session: RelaySession, key: string) { + const byKey = missedRecovery.get(session); const recovered = byKey?.get(key); byKey?.delete(key); return recovered; @@ -138,16 +140,14 @@ function notifyRecovered( key: string, recovered: RecoveredSend, ) { - const listeners = recoveryListeners.get(session)?.get(key); - if (recovered.preparationError !== undefined) { - let byKey = missedPreparation.get(session); - if (!byKey) { - byKey = new Map(); - missedPreparation.set(session, byKey); - } - byKey.set(key, recovered); + let byKey = missedRecovery.get(session); + if (!byKey) { + byKey = new Map(); + missedRecovery.set(session, byKey); } - for (const listener of listeners ?? []) listener(recovered); + byKey.set(key, recovered); + for (const listener of recoveryListeners.get(session)?.get(key) ?? []) + listener(recovered); } function errorForRecovery(reason: unknown) { return reason instanceof Error @@ -605,45 +605,63 @@ function Composer({ setConflict(false); setStorageFailed(false); } - const reconcileDraft = useEffectEvent((restored?: MentionDraft) => { - if (submission || editing.target || writing.current) return; - if (accepted) { - // The recovered revision can precede attachment adoption. The explicit - // recovery notification below reconciles the mounted owner afterward. - if (recoveryFor(session).get(recoveryKey) === accepted) return; + // Picks belong to the recovered revision alone. They return whenever the + // composer holds it unedited, never to a later edit that keeps a conflict. + const reconcileDraft = useEffectEvent( + (restored?: MentionDraft, recoveredPicks?: MentionPicks) => { + if (submission || editing.target || writing.current) return; + if (accepted) { + // The recovered revision can precede attachment adoption. The explicit + // recovery notification below reconciles the mounted owner afterward. + if (recoveryFor(session).get(recoveryKey) === accepted) return; + const current = viewRevision(scope, draftKey); + if (current !== undefined) { + setAccepted(undefined); + loadSaved(current); + if (restored && current === JSON.stringify(restored)) + setPicks(recoveredPicks ?? {}); + } + return; + } const current = viewRevision(scope, draftKey); - if (current !== undefined) { - setAccepted(undefined); - loadSaved(current); + if (current === undefined) return; + const recovering = !!restored && current === JSON.stringify(restored); + if (current === revision.current) { + // The storage listener usually loads the recovered revision before + // its notification arrives. + if (recovering && !dirty.current) setPicks(recoveredPicks ?? {}); + return; } - return; - } - const current = viewRevision(scope, draftKey); - if (current === undefined || current === revision.current) return; - // An explicit successful recovery may replace only the cleared follow-up, - // never a meaningful unsaved edit. Ordinary external revisions keep their - // conflict behavior. - if ( - restored && - current === JSON.stringify(restored) && - !valueRef.current.text.trim() && - !valueRef.current.recipients.length && - !composerMarkdown(valueRef.current).trim() && - !sendAttempt.current - ) - loadSaved(current); - else if (dirty.current || sendAttempt.current) setConflict(true); - else loadSaved(current); - }); + // An explicit successful recovery may replace only the cleared follow-up, + // never a meaningful unsaved edit. Ordinary external revisions keep their + // conflict behavior. + if ( + recovering && + !valueRef.current.text.trim() && + !valueRef.current.recipients.length && + !composerMarkdown(valueRef.current).trim() && + !sendAttempt.current + ) + loadSaved(current); + else if (dirty.current || sendAttempt.current) { + setConflict(true); + return; + } else loadSaved(current); + if (recovering) setPicks(recoveredPicks ?? {}); + }, + ); const editingDraft = !!editing.target; const deferredRecovery = useRef(undefined); useEffect(() => { if (submission) return; const stop = subscribeView(scope, () => reconcileDraft()); // Reconcile changes while message-edit mode or mounting paused this listener. - reconcileDraft(deferredRecovery.current?.draft); + reconcileDraft( + deferredRecovery.current?.draft, + deferredRecovery.current?.picks, + ); if (!editingDraft && deferredRecovery.current) { - takeMissedPreparation(session, recoveryKey); + takeMissedRecovery(session, recoveryKey); setError(errorForRecovery(deferredRecovery.current.preparationError)); deferredRecovery.current = undefined; } @@ -651,9 +669,9 @@ function Composer({ }, [scope, submission, editingDraft, session, recoveryKey]); const receiveRecovery = useEffectEvent((recovered: RecoveredSend) => { if (editing.target) deferredRecovery.current = recovered; - reconcileDraft(recovered.draft); + reconcileDraft(recovered.draft, recovered.picks); if (!editing.target) { - takeMissedPreparation(session, recoveryKey); + takeMissedRecovery(session, recoveryKey); setError(errorForRecovery(recovered.preparationError)); } }); @@ -664,11 +682,11 @@ function Composer({ // Recovery may have completed between unmount and this subscription. Replay // only if its saved caption still owns the draft; never attach an old error // to a replacement written while the composer was away. - const missed = missedPreparation.get(session)?.get(recoveryKey); + const missed = missedRecovery.get(session)?.get(recoveryKey); if (missed) { if (viewRevision(scope, draftKey) === JSON.stringify(missed.draft)) receiveRecovery(missed); - else takeMissedPreparation(session, recoveryKey); + else takeMissedRecovery(session, recoveryKey); } return stop; }, [session, recoveryKey, scope, draftKey]); @@ -1257,8 +1275,11 @@ function Composer({ return false; if (!target.adopt(files)) return false; recoveryFor(session).delete(recoveryKey); + // The whole captured pick set returns with the whole draft; the + // recipient filter above applies again on the next send. notifyRecovered(session, recoveryKey, { draft: captured, + picks: capturedPicks, preparationError, }); return true;