From 12c6b45d1bfc27ea1620d57cd20c2c7a0569f0c2 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 12 Oct 2026 16:34:08 +1100 Subject: [PATCH 1/3] fix(composer): restore model and effort picks when a background send is recovered A failed or cancelled attachment send restores its draft and files but not the model/effort picks captured for it, so a resend went out at the agents' defaults while the picker showed unpicked. Carry the captured picks in the recovery record and apply them wherever the composer ends up holding the recovered revision unedited: the accepted-draft load, the explicit recovery load, and the usual case where the storage listener has already loaded it before its notification arrives. Conflict paths never take them. Picks are still not persisted with the draft and do not survive a reload. Follow-up to #856: https://github.com/block/buzz-app/pull/856#discussion_r4243810747 Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- .../messages/MessageComposer.test.tsx | 156 ++++++++++++++++-- src/features/messages/MessageComposer.tsx | 87 ++++++---- 2 files changed, 200 insertions(+), 43 deletions(-) diff --git a/src/features/messages/MessageComposer.test.tsx b/src/features/messages/MessageComposer.test.tsx index 0b0c20f6d1..123bf5a324 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,124 @@ it("restores a failed background upload into the composer with Desktop's toast", ); }); +it.each([false, true])( + "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) { + h.unmount(); + container = render( + + + , + { wrapper: ToastProvider }, + ).container; + } + await act(async () => { + h.uploadCalls[0]?.result.reject(new Error("offline")); + }); + 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..6742751c2d 100644 --- a/src/features/messages/MessageComposer.tsx +++ b/src/features/messages/MessageComposer.tsx @@ -116,7 +116,11 @@ 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>> @@ -605,43 +609,61 @@ 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, picks?: 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(picks ?? {}); + } + 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(picks ?? {}); + 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(picks ?? {}); + }, + ); 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); setError(errorForRecovery(deferredRecovery.current.preparationError)); @@ -651,7 +673,7 @@ 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); setError(errorForRecovery(recovered.preparationError)); @@ -1257,8 +1279,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; From ae59c11bf74458e51cc2009b5736f20a0f2f2e02 Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Mon, 12 Oct 2026 20:32:31 +1100 Subject: [PATCH 2/3] refactor(composer): name the recovered picks parameter distinctly The reconcileDraft parameter shadowed the picks state declared earlier in the component; capturedPicks reads the state, so one identifier meant two things. Rename the parameter to recoveredPicks. No behavior change. Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- src/features/messages/MessageComposer.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/features/messages/MessageComposer.tsx b/src/features/messages/MessageComposer.tsx index 6742751c2d..1835106635 100644 --- a/src/features/messages/MessageComposer.tsx +++ b/src/features/messages/MessageComposer.tsx @@ -612,7 +612,7 @@ function Composer({ // 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, picks?: MentionPicks) => { + (restored?: MentionDraft, recoveredPicks?: MentionPicks) => { if (submission || editing.target || writing.current) return; if (accepted) { // The recovered revision can precede attachment adoption. The explicit @@ -623,7 +623,7 @@ function Composer({ setAccepted(undefined); loadSaved(current); if (restored && current === JSON.stringify(restored)) - setPicks(picks ?? {}); + setPicks(recoveredPicks ?? {}); } return; } @@ -633,7 +633,7 @@ function Composer({ if (current === revision.current) { // The storage listener usually loads the recovered revision before // its notification arrives. - if (recovering && !dirty.current) setPicks(picks ?? {}); + if (recovering && !dirty.current) setPicks(recoveredPicks ?? {}); return; } // An explicit successful recovery may replace only the cleared follow-up, @@ -651,7 +651,7 @@ function Composer({ setConflict(true); return; } else loadSaved(current); - if (recovering) setPicks(picks ?? {}); + if (recovering) setPicks(recoveredPicks ?? {}); }, ); const editingDraft = !!editing.target; From 0716636a0e8db5ba87341b1c59433fd7c0101bfb Mon Sep 17 00:00:00 2001 From: Matt Toohey Date: Tue, 13 Oct 2026 09:12:34 +1100 Subject: [PATCH 3/3] fix(composer): retain recovered picks until an unmounted composer returns Recovery records were retained for replay only when they carried a preparation error, so an ordinary upload failure or cancel that landed while the composer was unmounted reached no listener and its pick snapshot was dropped; on return the caption and files came back from storage but the picker showed unpicked and a retry used the defaults. Retain every recovery record that no mounted composer has consumed. The existing remount replay still applies it only while the saved draft is the recovered revision and retires it otherwise, and consumption clears it as before. Add the unmount -> fail -> remount case, which fails without this change. https://github.com/block/buzz-app/pull/887#discussion_r4245541583 Co-Authored-By: Claude Fable 5.1 Signed-off-by: Matt Toohey --- .../messages/MessageComposer.test.tsx | 12 ++++-- src/features/messages/MessageComposer.tsx | 38 +++++++++---------- 2 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/features/messages/MessageComposer.test.tsx b/src/features/messages/MessageComposer.test.tsx index 123bf5a324..d5e62daf07 100644 --- a/src/features/messages/MessageComposer.test.tsx +++ b/src/features/messages/MessageComposer.test.tsx @@ -1225,7 +1225,9 @@ it("restores a failed background upload into the composer with Desktop's toast", ); }); -it.each([false, true])( +// 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. @@ -1292,8 +1294,8 @@ it.each([false, true])( await waitFor(() => expect(h.uploadCalls).toHaveLength(1)); let container = h.container; - if (remounted) { - h.unmount(); + 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(); diff --git a/src/features/messages/MessageComposer.tsx b/src/features/messages/MessageComposer.tsx index 1835106635..2e0722ffc0 100644 --- a/src/features/messages/MessageComposer.tsx +++ b/src/features/messages/MessageComposer.tsx @@ -125,14 +125,12 @@ 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; @@ -142,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 @@ -665,7 +661,7 @@ function Composer({ deferredRecovery.current?.picks, ); if (!editingDraft && deferredRecovery.current) { - takeMissedPreparation(session, recoveryKey); + takeMissedRecovery(session, recoveryKey); setError(errorForRecovery(deferredRecovery.current.preparationError)); deferredRecovery.current = undefined; } @@ -675,7 +671,7 @@ function Composer({ if (editing.target) deferredRecovery.current = recovered; reconcileDraft(recovered.draft, recovered.picks); if (!editing.target) { - takeMissedPreparation(session, recoveryKey); + takeMissedRecovery(session, recoveryKey); setError(errorForRecovery(recovered.preparationError)); } }); @@ -686,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]);