From 2d9eccba0b172f4003b33ac87e47518d6bcbd0f4 Mon Sep 17 00:00:00 2001 From: morgmart <98432065+morgmart@users.noreply.github.com> Date: Fri, 30 Oct 2026 20:40:03 -0700 Subject: [PATCH 1/3] fix(navigation): skip empty new chats in Back history A New chat or New message page that holds nothing the person wrote is not a place to come back to. The page reports this to navigation, and the next page opened from it takes its place in history. A draft with text, an attached chat, or files stays a Back stop, using the same draft definition as the Drafts list. A new chat's default agent tag is shown without being saved. Settings never replaces the visit it returns to, and returning to an empty visit's opener reuses that entry. Co-authored-by: Carl Co-Authored-By: Claude Opus 5.5 Signed-off-by: morgmart <98432065+morgmart@users.noreply.github.com> --- src/bundled/agents/AgentsPage.test.tsx | 2 + .../channels/ChannelsPage.conversion.test.tsx | 1 + .../channels/ChannelsPage.solo.test.tsx | 177 +++++++++++++++++- .../channels/ChannelsPage.tabs.test.tsx | 1 + src/bundled/channels/ChannelsPage.tsx | 7 +- src/bundled/relay-notes/NotesPage.test.tsx | 1 + .../NewMessage.recovery.test.tsx | 31 ++- .../direct-messages/NewMessage.test.tsx | 56 +++++- src/features/direct-messages/NewMessage.tsx | 21 ++- src/features/messages/MessageComposer.tsx | 8 +- src/features/messages/ThreadPanel.test.tsx | 1 + .../messages/attachment-panes.test.tsx | 1 + src/features/messages/draft-visit.ts | 44 +++++ src/features/navigation/controller.test.ts | 66 +++++++ src/features/navigation/controller.ts | 31 ++- src/features/navigation/service.ts | 7 + src/features/solo/SoloDraft.tsx | 23 ++- 17 files changed, 464 insertions(+), 14 deletions(-) create mode 100644 src/features/messages/draft-visit.ts diff --git a/src/bundled/agents/AgentsPage.test.tsx b/src/bundled/agents/AgentsPage.test.tsx index c530b1c9ce..43d3dfc72b 100644 --- a/src/bundled/agents/AgentsPage.test.tsx +++ b/src/bundled/agents/AgentsPage.test.tsx @@ -676,6 +676,7 @@ it("restores provider links and retires only that provider's visits after close signal: new AbortController().signal, focus: "content" as const, forSession: () => navigation, + passThrough: () => () => {}, resolve: () => true, complete: () => true, }; @@ -749,6 +750,7 @@ it("opens the agent a link names, on Your agents", async () => { forSession: () => navigation, resolve: () => true, complete: () => true, + passThrough: () => () => {}, }; setup([agent("Ada", 1), agent("Bea", 2)], { page: { navigator, navigation }, diff --git a/src/bundled/channels/ChannelsPage.conversion.test.tsx b/src/bundled/channels/ChannelsPage.conversion.test.tsx index 9d5c25bddf..c1a66136f7 100644 --- a/src/bundled/channels/ChannelsPage.conversion.test.tsx +++ b/src/bundled/channels/ChannelsPage.conversion.test.tsx @@ -170,6 +170,7 @@ function fixture() { }, signal: visit.signal, forSession: () => navigation, + passThrough: () => () => {}, complete: () => true, resolve: () => true, }; diff --git a/src/bundled/channels/ChannelsPage.solo.test.tsx b/src/bundled/channels/ChannelsPage.solo.test.tsx index fdf9473dab..2aedcadc79 100644 --- a/src/bundled/channels/ChannelsPage.solo.test.tsx +++ b/src/bundled/channels/ChannelsPage.solo.test.tsx @@ -26,6 +26,8 @@ import { soloDraftTarget, } from "../../features/channel-navigation/routes"; import { linkedDraftKey } from "../../features/solo/linked-chat"; +import { attachmentDraft } from "../../features/messages/attachment-draft"; +import type { ComposerSession } from "../../features/messages/composer-session"; import { readView, writeView } from "../../shared/view-state"; import { ChannelsPage, type ArchiveShortcut } from "./ChannelsPage"; import { SoloWorkspaces } from "./SoloWorkspaces"; @@ -56,9 +58,12 @@ function fixture({ archived = false, viewerNamed = false, others = [], + agentless = false, }: { /** Other Solo chats listed after this one in the sidebar's Chats. */ others?: readonly string[]; + /** No agent can be tagged, so nothing is tagged automatically. */ + agentless?: boolean; /** The viewer's own profile has loaded too. */ viewerNamed?: boolean; /** Someone other than the viewer or its agent created the chat. */ @@ -145,10 +150,12 @@ function fixture({ }, ); // The default agent comes first, so a named agent is distinguishable from it. - const agents = [ - { pubkey: "b".repeat(64), name: "Helper", managed: false }, - { pubkey: agent, name: "Scout", managed: false }, - ]; + const agents = agentless + ? [] + : [ + { pubkey: "b".repeat(64), name: "Helper", managed: false }, + { pubkey: agent, name: "Scout", managed: false }, + ]; const choices = { ...owner.session.agentChoices.snapshot(), status: "ready" as const, @@ -286,6 +293,7 @@ function fixture({ target: target?.(scope) ?? conversationTarget(scope, id), signal: abort.signal, forSession: () => navigation, + passThrough: vi.fn(() => () => {}), complete, resolve: () => true, }; @@ -335,6 +343,7 @@ function fixture({ ); return { + navigation, mount: () => render(page()), mountWithSidebar: () => render(page(true)), mountWithToasts: () => render({page()}), @@ -510,6 +519,166 @@ it("addresses a Solo draft route's agent in the new chat without tagging the tex expect(f.send).toHaveBeenCalledWith(expect.any(String), "Hello", [agent]); }); +it("keeps a new chat out of Back history until something is written in it", async () => { + const f = fixture({ target: (scope) => soloDraftTarget(scope, { agent }) }); + f.mount(); + const region = await screen.findByRole("region", { name: "New chat" }); + const input = within(region).getByRole("textbox", { + name: "Message your agents", + }); + await waitFor(() => expect(input).toHaveTextContent("@Scout")); + const passes = vi.mocked(f.navigation.passThrough).mock.lastCall?.[0]; + // Only the automatic tag, shown but not saved: nothing to come back to. + expect(readView(f.scope, "solo:new-draft", null)).toBeNull(); + expect(passes?.()).toBe(true); + const user = userEvent.setup(); + await user.click(input); + await user.keyboard(" Plan the week"); + expect(passes?.()).toBe(false); + await user.click( + within(region).getByRole("button", { name: "Send message" }), + ); + await waitFor(() => expect(f.open).toHaveBeenCalledOnce()); + // The sent draft is spent; the started chat takes its place. + expect(passes?.()).toBe(true); +}); + +it("keeps a written new chat in Back history when its draft cannot be saved", async () => { + const f = fixture({ target: (scope) => soloDraftTarget(scope, { agent }) }); + const draftStorageKey = `buzz-view.v1:${JSON.stringify([f.scope, "solo:new-draft"])}`; + const setItem = Storage.prototype.setItem; + vi.spyOn(Storage.prototype, "setItem").mockImplementation(function ( + this: Storage, + key: string, + value: string, + ) { + if (key === draftStorageKey) + throw new DOMException("Storage is full", "QuotaExceededError"); + setItem.call(this, key, value); + }); + try { + f.mount(); + const region = await screen.findByRole("region", { name: "New chat" }); + const input = within(region).getByRole("textbox", { + name: "Message your agents", + }); + await waitFor(() => expect(input).toHaveTextContent("@Scout")); + const passes = vi.mocked(f.navigation.passThrough).mock.lastCall?.[0]; + expect(passes?.()).toBe(true); + const user = userEvent.setup(); + await user.click(input); + await user.keyboard(" Unsaved but written"); + expect(readView(f.scope, "solo:new-draft", null)).toBeNull(); + expect(passes?.()).toBe(false); + } finally { + vi.mocked(Storage.prototype.setItem).mockRestore(); + } +}); + +it("keeps a saved draft that only tags the default agent in Back history", async () => { + const f = fixture({ target: (scope) => soloDraftTarget(scope, { agent }) }); + // A tag the person typed, saved with its editor snapshot. + const typed = { + text: "@Scout ", + recipients: [{ pubkey: agent, name: "Scout", start: 0, end: 6 }], + document: { + version: 1, + content: { + type: "doc", + content: [ + { + type: "paragraph", + content: [ + { + type: "text", + text: "@Scout", + marks: [ + { + type: "recipient", + attrs: { pubkey: agent, name: "Scout" }, + }, + ], + }, + { type: "text", text: " " }, + ], + }, + ], + }, + }, + }; + writeView(f.scope, "solo:new-draft", typed); + f.mount(); + const region = await screen.findByRole("region", { name: "New chat" }); + await waitFor(() => + expect( + within(region).getByRole("textbox", { name: "Message your agents" }), + ).toHaveTextContent("@Scout"), + ); + const passes = vi.mocked(f.navigation.passThrough).mock.lastCall?.[0]; + expect(passes?.()).toBe(false); + expect(readView(f.scope, "solo:new-draft", null)).toEqual(typed); +}); + +it("keeps a linked chat draft in Back history, since it holds its source chat", async () => { + const linked = "22222222-2222-4222-8222-222222222222"; + const f = fixture({ target: (scope) => soloDraftTarget(scope, { linked }) }); + writeView(f.scope, linkedDraftKey(linked), { + channelId: "33333333-3333-4333-8333-333333333333", + link: `buzz://message?channel=33333333-3333-4333-8333-333333333333&id=${"e".repeat(64)}`, + }); + f.mount(); + await screen.findByRole("textbox"); + expect(vi.mocked(f.navigation.passThrough).mock.lastCall?.[0]()).toBe(false); +}); + +it("keeps an untagged linked chat draft in Back history until its source is used", async () => { + const linked = "22222222-2222-4222-8222-222222222222"; + const f = fixture({ + agentless: true, + target: (scope) => soloDraftTarget(scope, { linked }), + }); + const link = `buzz://message?channel=33333333-3333-4333-8333-333333333333&id=${"e".repeat(64)}`; + writeView(f.scope, linkedDraftKey(linked), { + channelId: "33333333-3333-4333-8333-333333333333", + link, + }); + f.mount(); + await waitFor(() => + expect(screen.getByRole("textbox")).toHaveTextContent(link), + ); + const passes = vi.mocked(f.navigation.passThrough).mock.lastCall?.[0]; + expect( + readView(f.scope, `${linkedDraftKey(linked)}:new-draft`, null), + ).toBeNull(); + expect(passes?.()).toBe(false); + // As a successful start leaves it: source consumed, draft absent. + writeView(f.scope, linkedDraftKey(linked), null); + expect(passes?.()).toBe(true); +}); + +it("counts a new chat's attached file as written", async () => { + const f = fixture({ target: soloDraftTarget }); + f.mount(); + const region = await screen.findByRole("region", { name: "New chat" }); + const passes = vi.mocked(f.navigation.passThrough).mock.lastCall?.[0]; + expect(passes?.()).toBe(true); + Object.assign(f.session, { attachments: { uploadForNewChat: vi.fn() } }); + attachmentDraft( + f.session as unknown as ComposerSession, + `${f.scope}:solo:new-draft`, + "", + true, + ).addUploaded({ + name: "notes.txt", + url: "https://media.invalid/notes.txt", + type: "text/plain", + size: 5, + sha256: "f".repeat(64), + }); + expect(passes?.()).toBe(false); + expect(region).toBeVisible(); +}); + it("a new Solo chat finishing behind Settings stays put and retires its draft", async () => { const f = fixture({ behindSettings: true, target: soloDraftTarget }); writeView(f.scope, "solo:new-draft", { diff --git a/src/bundled/channels/ChannelsPage.tabs.test.tsx b/src/bundled/channels/ChannelsPage.tabs.test.tsx index 374961fc2c..c17768e8f2 100644 --- a/src/bundled/channels/ChannelsPage.tabs.test.tsx +++ b/src/bundled/channels/ChannelsPage.tabs.test.tsx @@ -215,6 +215,7 @@ async function page( }, resolve: () => true, complete: () => true, + passThrough: () => () => {}, } : undefined; render( diff --git a/src/bundled/channels/ChannelsPage.tsx b/src/bundled/channels/ChannelsPage.tsx index ee6e4c2ea8..d63c7cd118 100644 --- a/src/bundled/channels/ChannelsPage.tsx +++ b/src/bundled/channels/ChannelsPage.tsx @@ -1946,6 +1946,7 @@ function ChannelWorkspace({ host={{ relay, panels, navigation: navigator }} session={queries} scope={scope} + visit={navigation} extensions={extensions} onPreparing={(pubkeys) => handoff?.prepareDm(pubkeys)} onOpenThread={openChannelThread} @@ -1972,6 +1973,7 @@ function ChannelWorkspace({ ])} linkedId={draft.linked} source={linkedSource} + visit={navigation} focusRequest={ navigation?.focus === "preserve" || (retiredSoloDraft.generation > 0 && @@ -2000,7 +2002,10 @@ function ChannelWorkspace({ navigator && !behindSettings() ) - void navigator.open(soloDraftTarget(scope, draft)); + // The chat was never made: its draft takes its place. + void navigator.open(soloDraftTarget(scope, draft), { + replace: true, + }); }} /> ) : ( diff --git a/src/bundled/relay-notes/NotesPage.test.tsx b/src/bundled/relay-notes/NotesPage.test.tsx index 70a6da573a..596c03d541 100644 --- a/src/bundled/relay-notes/NotesPage.test.tsx +++ b/src/bundled/relay-notes/NotesPage.test.tsx @@ -315,6 +315,7 @@ it.each(["icons", "", "two\nlines"])( target: snapshot.entry.target, signal: snapshot.attempt.signal, forSession: () => request, + passThrough: () => () => {}, resolve: () => false, complete: (result) => controller.complete(snapshot.attempt, result), }; diff --git a/src/features/direct-messages/NewMessage.recovery.test.tsx b/src/features/direct-messages/NewMessage.recovery.test.tsx index 77f2e28311..04704d5a92 100644 --- a/src/features/direct-messages/NewMessage.recovery.test.tsx +++ b/src/features/direct-messages/NewMessage.recovery.test.tsx @@ -42,6 +42,7 @@ import { MentionCompletion } from "../../bundled/mentions/MentionCompletion"; import { mentionQuery } from "../../bundled/mentions/mention-query"; import { attachmentDraft } from "../messages/attachment-draft"; import { NewMessage } from "./NewMessage"; +import type { PageNavigation } from "../navigation/service"; import { OutboxStatus } from "../../bundled/channels/OutboxStatus"; import { createRelayProfiler } from "../relay/profiling"; import { readView, writeView } from "../../shared/view-state"; @@ -189,9 +190,11 @@ function setup( function Draft({ owner, extensions, + visit, }: { owner: ReturnType; extensions?: ConversationExtensions | undefined; + visit?: PageNavigation | undefined; }) { const { channels } = useChannelList(owner.session.channels); return ( @@ -219,13 +222,15 @@ function setup( onOpenThread={vi.fn()} onStarted={onStarted} extensions={extensions} + visit={visit} /> ); } const mount = ( owner: ReturnType, extensions?: ConversationExtensions, - ) => render(); + visit?: PageNavigation, + ) => render(); const user = userEvent.setup(); const compose = async () => { await user.click(await screen.findByRole("option", { name: "Avery" })); @@ -431,6 +436,30 @@ it("removing a failed operation in Diagnostics unlocks the preserved draft on re expect(t.publish).toHaveBeenCalledTimes(2); }); +it("keeps a failed message in Back history when only the outbox holds it", async () => { + const t = setup(); + t.publish.mockRejectedValueOnce(new PublishRejected("Not sent")); + const owner = t.create(); + const outbox = owner.session.outbox; + assert.exists(outbox); + const page = t.mount(owner); + await t.compose(); + await t.user.click(send()); + await screen.findByRole("alert"); + await waitFor(() => expect(outbox.snapshot()[0]?.delivery).toBe("failed")); + page.unmount(); + // The disposable draft view is gone; the durable outbox entry remains. + writeView(scope, "direct-message:new-draft", null); + const passThrough = vi.fn(() => () => {}); + t.mount(owner, undefined, { passThrough } as unknown as PageNavigation); + await waitFor(() => + expect(screen.getByRole("textbox")).toHaveTextContent( + "Durable first message", + ), + ); + expect(passThrough.mock.lastCall?.[0]()).toBe(false); +}); + it("never publishes if the durable recovery write fails", async () => { const t = setup(); t.storage.save = async () => { diff --git a/src/features/direct-messages/NewMessage.test.tsx b/src/features/direct-messages/NewMessage.test.tsx index 53a61c345b..be8559ee91 100644 --- a/src/features/direct-messages/NewMessage.test.tsx +++ b/src/features/direct-messages/NewMessage.test.tsx @@ -13,6 +13,7 @@ import { waitFor, within, } from "@testing-library/react"; +import type { PageNavigation } from "../navigation/service"; import userEvent from "@testing-library/user-event"; import type { ChannelSummary } from "../relay/contracts"; import type { OutgoingEvent } from "../relay/outbox"; @@ -207,7 +208,7 @@ function setup() { signal: new AbortController().signal, }), }); - const mount = () => + const mount = (visit?: PageNavigation) => render( , @@ -705,6 +707,58 @@ it("reveals the highlighted person on every arrow key and when the list reopens" ); }); +it("stays out of Back history until a message is written, whoever it addresses", async () => { + const t = setup(); + const passThrough = vi.fn(() => () => {}); + t.mount({ passThrough } as unknown as PageNavigation); + const passes = passThrough.mock.lastCall?.[0]; + expect(passes?.()).toBe(true); + await t.user.click(await screen.findByRole("option", { name: "Person 1" })); + expect(passes?.()).toBe(true); + await t.user.type(screen.getByRole("textbox"), "Hello"); + expect(passes?.()).toBe(false); +}); + +it("stays in Back history once a message is written, even if it cannot be saved", async () => { + const draftStorageKey = `buzz-view.v1:${JSON.stringify([scope, "direct-message:new-draft"])}`; + const setItem = Storage.prototype.setItem; + vi.spyOn(Storage.prototype, "setItem").mockImplementation(function ( + this: Storage, + key: string, + value: string, + ) { + if (key === draftStorageKey) + throw new DOMException("Storage is full", "QuotaExceededError"); + setItem.call(this, key, value); + }); + try { + const t = setup(); + const passThrough = vi.fn(() => () => {}); + t.mount({ passThrough } as unknown as PageNavigation); + const passes = passThrough.mock.lastCall?.[0]; + await t.user.click(await screen.findByRole("option", { name: "Person 1" })); + await t.user.type(screen.getByRole("textbox"), "Hello"); + expect(localStorage.getItem(draftStorageKey)).toBeNull(); + expect(passes?.()).toBe(false); + } finally { + vi.mocked(Storage.prototype.setItem).mockRestore(); + } +}); + +it("leaves Back history once its written message is sent", async () => { + const t = setup(); + const passThrough = vi.fn(() => () => {}); + t.mount({ passThrough } as unknown as PageNavigation); + const passes = passThrough.mock.lastCall?.[0]; + await t.user.click(await screen.findByRole("option", { name: "Person 1" })); + await t.user.type(screen.getByRole("textbox"), "Hello"); + expect(passes?.()).toBe(false); + await t.user.click(send()); + await waitFor(() => expect(t.onStarted).toHaveBeenCalledOnce()); + // The sent message is spent; the conversation it started takes its place. + expect(passes?.()).toBe(true); +}); + it("opens blank and focused, adds multiple recipients, deduplicates, and enforces eight", async () => { const t = setup(); t.directMessages.people.mockResolvedValue({ diff --git a/src/features/direct-messages/NewMessage.tsx b/src/features/direct-messages/NewMessage.tsx index 53fd4e18fb..f177164d45 100644 --- a/src/features/direct-messages/NewMessage.tsx +++ b/src/features/direct-messages/NewMessage.tsx @@ -33,6 +33,8 @@ import { clearView, readView, writeView } from "../../shared/view-state"; import { Button } from "../../shared/design-system/ui/Button"; import { RecipientPicker } from "./RecipientPicker"; import type { Recipient } from "./usePeople"; +import { usePassThroughUntilDrafted } from "../messages/draft-visit"; +import type { PageNavigation } from "../navigation/service"; import styles from "./NewMessage.module.css"; const draftKey = "direct-message:new-draft"; @@ -135,9 +137,12 @@ export function NewMessage({ host, onOpenThread, onStarted, + visit, }: { session: RelaySession; scope: string; + /** The visit presenting this composer, a Back stop only once drafted. */ + visit?: PageNavigation | undefined; extensions?: ConversationExtensions | undefined; channels: readonly ChannelSummary[]; host: ConversationHostServices; @@ -150,6 +155,17 @@ export function NewMessage({ ): void; onStarted(channelId: string, messageId: string): void; }) { + // A message held for delivery or retry is shown even when its draft view is + // gone, so it counts as drafted too. + const held = useRef(undefined); + const heldDraft = useCallback(() => held.current, []); + const draftEdited = usePassThroughUntilDrafted( + visit, + session, + scope, + draftKey, + heldDraft, + ); const [recipients, setRecipients] = useState(() => savedRecipients(scope, session.viewer), ); @@ -188,6 +204,7 @@ export function NewMessage({ } catch { /* Readiness presents invalid recovery. */ } + held.current = pending?.draft; const failed = pending && session.directMessages.delivery(pending.id) === "failed"; const locked = !ready || !!busy || (!!pending && !failed); @@ -195,10 +212,11 @@ export function NewMessage({ clearAttachmentDraft(session, `${scope}:${draftKey}`); retiring.current = undefined; prepared.current = undefined; + draftEdited(undefined); setRecipients([]); setRestoredDraft(mentionDraft("")); setDraftRevision((revision) => revision + 1); - }, [session, scope]); + }, [session, scope, draftEdited]); useLayoutEffect(() => { const confirmed = pending && session.directMessages.delivery(pending.id); if (pending) { @@ -539,6 +557,7 @@ export function NewMessage({ attachments: true, allowAttachmentOnly: true, recoveredDraft: restoredDraft, + onDraftChange: draftEdited, locked, disabled: !ready || diff --git a/src/features/messages/MessageComposer.tsx b/src/features/messages/MessageComposer.tsx index ecbd2b5d92..5edc9de422 100644 --- a/src/features/messages/MessageComposer.tsx +++ b/src/features/messages/MessageComposer.tsx @@ -322,6 +322,8 @@ export type MessageComposerProps = { attachments?: boolean; /** DMs can start with files alone; named Solo chats still require text. */ allowAttachmentOnly?: boolean; + /** Hears each edit, whether or not saving it succeeded. */ + onDraftChange?: ((draft: MentionDraft) => void) | undefined; /** `addressed`: the toolbar's agents, when they are held apart from the text. */ submit: ( draft: MentionDraft, @@ -893,8 +895,10 @@ function Composer({ valueRef.current = next; updateDraft(next); if (save && !editing.target) { - if (submission) writeView(scope, draftKey, next); - else { + if (submission) { + writeView(scope, draftKey, next); + submission.onDraftChange?.(next); + } else { dirty.current = true; if (!conflict && !accepted) persist(next); } diff --git a/src/features/messages/ThreadPanel.test.tsx b/src/features/messages/ThreadPanel.test.tsx index 90dedc40b1..31143ac680 100644 --- a/src/features/messages/ThreadPanel.test.tsx +++ b/src/features/messages/ThreadPanel.test.tsx @@ -149,6 +149,7 @@ function ordinaryNavigation() { complete: vi.fn(() => true), resolve: vi.fn(() => true), forSession: vi.fn(), + passThrough: () => () => {}, } satisfies PageNavigation; } function messagesHarness( diff --git a/src/features/messages/attachment-panes.test.tsx b/src/features/messages/attachment-panes.test.tsx index 9f936cc138..19e95a26c1 100644 --- a/src/features/messages/attachment-panes.test.tsx +++ b/src/features/messages/attachment-panes.test.tsx @@ -207,6 +207,7 @@ it.each([false, true])( ), signal: new AbortController().signal, forSession: () => navigation, + passThrough: () => () => {}, complete: () => true, resolve: () => true, }; diff --git a/src/features/messages/draft-visit.ts b/src/features/messages/draft-visit.ts new file mode 100644 index 0000000000..a61298cdd3 --- /dev/null +++ b/src/features/messages/draft-visit.ts @@ -0,0 +1,44 @@ +import { useCallback, useEffect, useRef } from "react"; +import type { PageNavigation } from "../navigation/service"; +import { readView } from "../../shared/view-state"; +import { attachmentDraftCount } from "./attachment-draft"; +import type { ComposerSession } from "./composer-session"; +import { + hasDraftContent, + mentionDraft, + type MentionDraft, +} from "./mention-draft"; + +/** A new conversation's visit is a Back stop only once its composer draft + * holds something the person wrote; until then the next page replaces it. + * `initialDraft` reads what the composer shows while nothing is saved. + * Returns the composer's edit listener; the page passes `undefined` once it + * has spent the draft. */ +export function usePassThroughUntilDrafted( + visit: PageNavigation | undefined, + session: ComposerSession, + scope: string, + draftKey: string, + initialDraft?: () => MentionDraft | string | undefined, +) { + // The composer's latest edit stands for the saved draft, which may be + // missing when storage fails, until the page spends it. + const edited = useRef(undefined); + useEffect( + () => + visit?.passThrough( + () => + !hasDraftContent( + edited.current ?? + mentionDraft( + readView(scope, draftKey, initialDraft?.() ?? ""), + ), + attachmentDraftCount(session, `${scope}:${draftKey}`), + ), + ), + [visit, session, scope, draftKey, initialDraft], + ); + return useCallback((draft: MentionDraft | undefined) => { + edited.current = draft; + }, []); +} diff --git a/src/features/navigation/controller.test.ts b/src/features/navigation/controller.test.ts index 369b751e76..b18676253a 100644 --- a/src/features/navigation/controller.test.ts +++ b/src/features/navigation/controller.test.ts @@ -284,6 +284,72 @@ describe("navigation attempts over one history driver", () => { nav.forward(); expect(nav.snapshot().entry.target).toEqual(settings); }); + describe("a visit whose page passes through", () => { + const draft: OpenTarget = { ...page, pageId: "draft" }; + const other: OpenTarget = { ...page, pageId: "other" }; + function onDraft(passes: () => boolean) { + const t = setup(); + void t.navigation.open(page); + void t.navigation.open(draft); + const stop = t.host.passThrough(t.navigation.snapshot().entry.id, passes); + return { ...t, stop }; + } + it("is replaced by the next page while it holds nothing", () => { + const { navigation: nav } = onDraft(() => true); + void nav.open(other); + nav.back(); + expect(nav.snapshot().entry.target).toEqual(page); + }); + it("returns to its opener when that page opens again", () => { + const t = setup(); + const nav = t.navigation; + void nav.open(page); + const opener = nav.snapshot().entry; + void nav.open(draft); + t.host.passThrough(nav.snapshot().entry.id, () => true); + void nav.open(page); + expect(nav.snapshot().entry).toBe(opener); + nav.back(); + expect(nav.snapshot().entry.target).not.toEqual(page); + }); + it("stays a Back stop while it holds something", () => { + const { navigation: nav } = onDraft(() => false); + void nav.open(other); + nav.back(); + expect(nav.snapshot().entry.target).toEqual(draft); + }); + it("stays under Settings, which returns to it", () => { + const { navigation: nav } = onDraft(() => true); + void nav.open(settings); + nav.back(); + expect(nav.snapshot().entry.target).toEqual(draft); + }); + it("is still replaced after returning from Settings", () => { + const { navigation: nav } = onDraft(() => true); + void nav.open(settings); + nav.back(); + expect(nav.snapshot().canGoForward).toBe(true); + void nav.open(other); + nav.back(); + expect(nav.snapshot().entry.target).toEqual(page); + }); + it("stays when its page cannot answer", () => { + const { navigation: nav } = onDraft(() => { + throw new Error("unavailable"); + }); + void nav.open(other); + expect(nav.snapshot().entry.target).toEqual(other); + nav.back(); + expect(nav.snapshot().entry.target).toEqual(draft); + }); + it("stays once its page stops reporting", () => { + const { navigation: nav, stop } = onDraft(() => true); + stop(); + void nav.open(other); + nav.back(); + expect(nav.snapshot().entry.target).toEqual(draft); + }); + }); it("uses driver-initiated traversal and bounds the memory driver without inventing history", () => { const { navigation: nav, history } = setup(2); void nav.open(settings); diff --git a/src/features/navigation/controller.ts b/src/features/navigation/controller.ts index b727dcc16f..707908844d 100644 --- a/src/features/navigation/controller.ts +++ b/src/features/navigation/controller.ts @@ -109,6 +109,16 @@ export function createNavigationController( // while one is attached. let presenters = 0; let ingressRetry: (() => Promise) | undefined; + // Visits whose page currently reports nothing worth returning to. + const passing = new Map boolean>(); + // A page that cannot answer keeps its visit, never the opening. + const passes = (entryId: string) => { + try { + return passing.get(entryId)?.() === true; + } catch { + return false; + } + }; const listeners = new Set<() => void>(); // Commit state and capture each caller's result before invoking external code. // Reentrant opens run against committed state and own their own result; deferred @@ -390,9 +400,16 @@ export function createNavigationController( opening = true; openingFocus = options?.focus; returningFocus = undefined; + // A passing visit leaves as a dismissal does: back to its opener when + // that is the page opening, else replaced by it. Settings returns to + // that visit, so it never replaces it. Any forward entries stay. + const dismiss = + options?.dismiss || + (target.kind !== "settings" && + passes(history.snapshot().current.id)); const previous = history.snapshot().previous; if ( - options?.dismiss && + dismiss && previous && targetKey(previous.target) === targetKey(target) && history.returnTo(previous.id) @@ -400,7 +417,7 @@ export function createNavigationController( if (openingFocus) returningFocus = openingFocus; return active.promise; } - if (options?.replace || options?.dismiss) history.replace(target); + if (options?.replace || dismiss) history.replace(target); else { const { current, previous } = history.snapshot(); if ( @@ -538,6 +555,15 @@ export function createNavigationController( return true; }); }, + /** While registered, the page presenting visit `entryId` reports through + * `passes` whether it holds anything to come back to. When it holds + * nothing, the next page opened from it replaces it in history. */ + passThrough(entryId: string, passes: () => boolean) { + passing.set(entryId, passes); + return () => { + if (passing.get(entryId) === passes) passing.delete(entryId); + }; + }, /** Host-only traversal to a retained visit; does not retire dismissed content. */ returnTo(entryId: string) { return transaction(() => !disposed && history.returnTo(entryId)); @@ -578,6 +604,7 @@ export function createNavigationController( const operation = active; effects.push(() => operation.controller.abort(NAVIGATION_ABORTED)); listeners.clear(); + passing.clear(); history.dispose(); }); }, diff --git a/src/features/navigation/service.ts b/src/features/navigation/service.ts index 0d197d4cb1..d130eaffc8 100644 --- a/src/features/navigation/service.ts +++ b/src/features/navigation/service.ts @@ -22,6 +22,10 @@ export type PageNavigation = Readonly<{ forSession(relay: RelayData, connection: RelaySnapshot): PageNavigation; /** Resolve the pending visit's default destination without replacing its caller. */ resolve(target: OpenTarget): boolean; + /** While registered, `passes` says whether this visit holds nothing to come + * back to; then the next page opened from it takes its place in history. + * Returns the unregister function. */ + passThrough(passes: () => boolean): () => void; complete( result: Extract, ): boolean; @@ -91,6 +95,8 @@ export function provideNavigation( ? (attempt.focus ?? "content") : "preserve"; }, + passThrough: (passes) => + controller.passThrough(attempt.entry.id, passes), resolve: (target) => { if ( retainUnderSettings && @@ -207,6 +213,7 @@ function bindPresentation( }, resolve: (target) => valid() && parent.resolve(target), complete: (result) => valid() && parent.complete(result), + passThrough: (passes) => parent.passThrough(passes), forSession(relay, connection) { let connections = sessions.get(relay); if (!connections) { diff --git a/src/features/solo/SoloDraft.tsx b/src/features/solo/SoloDraft.tsx index 1b2ec22470..bd70dd7242 100644 --- a/src/features/solo/SoloDraft.tsx +++ b/src/features/solo/SoloDraft.tsx @@ -1,4 +1,4 @@ -import { linkedDraftKey, type LinkedChat } from "./linked-chat"; +import { linkedDraftKey, readLinkedChat, type LinkedChat } from "./linked-chat"; import { canonicalDetailsName, detailsDraftErrors, @@ -15,7 +15,7 @@ import { parseChatSetup, type ChatSetup, } from "./workspace"; -import { useEffect, useRef, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import type { RelaySession } from "../relay/session"; import { readView, @@ -42,6 +42,8 @@ import { type AttachmentDraft, } from "../messages/attachment-draft"; import type { ConversationExtensions } from "../conversation/contracts"; +import { usePassThroughUntilDrafted } from "../messages/draft-visit"; +import type { PageNavigation } from "../navigation/service"; import styles from "./Solo.module.css"; /** Starting a new chat from a project starts it in that project, even when an @@ -97,7 +99,10 @@ export function SoloDraft({ agent, linkedId, source, + visit, }: { + /** The visit presenting this draft, a Back stop only once drafted. */ + visit?: PageNavigation | undefined; linkedId?: string | undefined; source?: LinkedChat | undefined; /** Start with this agent chosen, as when search opens a new chat with it. */ @@ -120,6 +125,18 @@ export function SoloDraft({ // Before the composer reads it: a named agent replaces a saved draft that // only tags another agent, since that tag was the earlier default. useState(() => dropOtherAgentTag(scope, `${draftKey}:new-draft`, agent)); + // Read the source live: starting the chat consumes it. + const linkedSource = useCallback( + () => (source ? readLinkedChat(scope, draftKey)?.link : undefined), + [source, scope, draftKey], + ); + const draftEdited = usePassThroughUntilDrafted( + visit, + session, + scope, + `${draftKey}:new-draft`, + linkedSource, + ); const [settingsOpen, setSettingsOpen] = useState(false); // New chats tag their agent in the draft: the one the route names, else the // default. @@ -333,6 +350,7 @@ export function SoloDraft({ // next start from that source seeds its link again. writeView(scope, `${draftKey}:new-draft`, source ? null : ""); writeView(scope, `${draftKey}:new-project`, null); + draftEdited(undefined); onStarted(channelId); // Only a start that made no chat ends here; the chat and its messages // are dropped (see channelWrites.start). Any other miss leaves the chat @@ -392,6 +410,7 @@ export function SoloDraft({ locked: busy, disabled: busy || !available, attachments: true, + onDraftChange: draftEdited, submit: (draft, files, addressed) => { void start(draft, files, addressed); }, From 452fa7682e29ee0fea7d0cbc90142a61c466ce72 Mon Sep 17 00:00:00 2001 From: morgmart <98432065+morgmart@users.noreply.github.com> Date: Fri, 30 Oct 2026 20:53:38 -0700 Subject: [PATCH 2/3] test(channels): cover a failed Solo start replacing its chat in history When starting a new chat fails before the chat exists, its draft takes the chat's place, so Back reaches the draft's opener, not a chat that was never made. The Back history tests now expect the agent addressed from the toolbar rather than tagged in the text, as on main. Co-authored-by: Morgarita <6e967af659416160ab4f2fd56d74c8c6f273791ee7bfc9468a1be37c387947be@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: morgmart <98432065+morgmart@users.noreply.github.com> --- .../channels/ChannelsPage.solo.test.tsx | 58 ++++++++++++++++--- 1 file changed, 49 insertions(+), 9 deletions(-) diff --git a/src/bundled/channels/ChannelsPage.solo.test.tsx b/src/bundled/channels/ChannelsPage.solo.test.tsx index 2aedcadc79..4b1eab6985 100644 --- a/src/bundled/channels/ChannelsPage.solo.test.tsx +++ b/src/bundled/channels/ChannelsPage.solo.test.tsx @@ -12,6 +12,7 @@ import { import userEvent from "@testing-library/user-event"; import { afterEach, expect, it, vi } from "vitest"; import { createRelaySession } from "../../features/relay/session"; +import { ChatNotStarted } from "../../features/relay/channel-writes"; import { keypair, signed } from "../../features/relay/testing"; import type { RelayData } from "../../features/relay/service"; import type { ChannelSummary } from "../../features/relay/contracts"; @@ -288,7 +289,7 @@ function fixture({ }; const abort = new AbortController(); const complete = vi.fn(() => true); - const navigation: PageNavigation = { + let navigation: PageNavigation = { entryId: "visit", target: target?.(scope) ?? conversationTarget(scope, id), signal: abort.signal, @@ -342,11 +343,18 @@ function fixture({ /> ); + let rendered: ReturnType | undefined; return { navigation, mount: () => render(page()), + mountWithToasts: () => + (rendered = render({page()})), + /** The navigator moved this page's visit to `next`. */ + show(next: OpenTarget) { + navigation = { ...navigation, target: next }; + rendered?.rerender({page()}); + }, mountWithSidebar: () => render(page(true)), - mountWithToasts: () => render({page()}), abort, archive, open, @@ -526,13 +534,14 @@ it("keeps a new chat out of Back history until something is written in it", asyn const input = within(region).getByRole("textbox", { name: "Message your agents", }); - await waitFor(() => expect(input).toHaveTextContent("@Scout")); + const user = userEvent.setup(); + await user.click(input); const passes = vi.mocked(f.navigation.passThrough).mock.lastCall?.[0]; - // Only the automatic tag, shown but not saved: nothing to come back to. + // The agent is addressed outside the text, and nothing is saved: nothing + // to come back to. + expect(input).not.toHaveTextContent("@"); expect(readView(f.scope, "solo:new-draft", null)).toBeNull(); expect(passes?.()).toBe(true); - const user = userEvent.setup(); - await user.click(input); await user.keyboard(" Plan the week"); expect(passes?.()).toBe(false); await user.click( @@ -543,6 +552,38 @@ it("keeps a new chat out of Back history until something is written in it", asyn expect(passes?.()).toBe(true); }); +it("gives a new chat that could not be started its draft's place in Back history", async () => { + const f = fixture({ target: (scope) => soloDraftTarget(scope, { agent }) }); + let refuse: (reason: unknown) => void = () => {}; + vi.mocked(f.session.channelWrites.start).mockImplementationOnce( + (chat: string) => ({ + create: f.session.channelWrites.create(chat), + ready: new Promise((_resolve, reject) => { + refuse = reject; + }), + }), + ); + f.mountWithToasts(); + const region = await screen.findByRole("region", { name: "New chat" }); + const user = userEvent.setup(); + await user.click( + within(region).getByRole("textbox", { name: "Message your agents" }), + ); + await user.keyboard(" Plan the week{Enter}"); + await waitFor(() => expect(f.open).toHaveBeenCalledOnce()); + const chat = vi.mocked(f.session.channelWrites.start).mock.lastCall?.[0]; + expect(chat).toBeDefined(); + f.show(conversationTarget(f.scope, chat ?? "")); + await act(async () => refuse(new ChatNotStarted("relay refused"))); + // Replacing the never-made chat leaves Back at the draft's opener. + await waitFor(() => + expect(f.open).toHaveBeenLastCalledWith( + soloDraftTarget(f.scope, { agent }), + { replace: true }, + ), + ); +}); + it("keeps a written new chat in Back history when its draft cannot be saved", async () => { const f = fixture({ target: (scope) => soloDraftTarget(scope, { agent }) }); const draftStorageKey = `buzz-view.v1:${JSON.stringify([f.scope, "solo:new-draft"])}`; @@ -562,11 +603,10 @@ it("keeps a written new chat in Back history when its draft cannot be saved", as const input = within(region).getByRole("textbox", { name: "Message your agents", }); - await waitFor(() => expect(input).toHaveTextContent("@Scout")); - const passes = vi.mocked(f.navigation.passThrough).mock.lastCall?.[0]; - expect(passes?.()).toBe(true); const user = userEvent.setup(); await user.click(input); + const passes = vi.mocked(f.navigation.passThrough).mock.lastCall?.[0]; + expect(passes?.()).toBe(true); await user.keyboard(" Unsaved but written"); expect(readView(f.scope, "solo:new-draft", null)).toBeNull(); expect(passes?.()).toBe(false); From 3394bd40f42ac7e803e3779b3ef69fe360e222c5 Mon Sep 17 00:00:00 2001 From: morgmart <98432065+morgmart@users.noreply.github.com> Date: Fri, 30 Oct 2026 21:53:30 -0700 Subject: [PATCH 3/3] fix(direct-messages): keep any held new message in Back history A pending send or retry is a Back stop whatever it carries, so a photo-only uncertain send restored after restart no longer drops its retry page from history. Also removes the inert agentless Solo fixture and its duplicate linked-draft case. Co-authored-by: Morgarita <6e967af659416160ab4f2fd56d74c8c6f273791ee7bfc9468a1be37c387947be@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 Signed-off-by: morgmart <98432065+morgmart@users.noreply.github.com> --- .../channels/ChannelsPage.solo.test.tsx | 30 ++++--------------- .../NewMessage.recovery.test.tsx | 8 +++-- src/features/direct-messages/NewMessage.tsx | 12 ++++---- src/features/messages/draft-visit.ts | 18 +++++++---- src/features/solo/SoloDraft.tsx | 2 +- 5 files changed, 31 insertions(+), 39 deletions(-) diff --git a/src/bundled/channels/ChannelsPage.solo.test.tsx b/src/bundled/channels/ChannelsPage.solo.test.tsx index 4b1eab6985..756b4c273b 100644 --- a/src/bundled/channels/ChannelsPage.solo.test.tsx +++ b/src/bundled/channels/ChannelsPage.solo.test.tsx @@ -59,12 +59,9 @@ function fixture({ archived = false, viewerNamed = false, others = [], - agentless = false, }: { /** Other Solo chats listed after this one in the sidebar's Chats. */ others?: readonly string[]; - /** No agent can be tagged, so nothing is tagged automatically. */ - agentless?: boolean; /** The viewer's own profile has loaded too. */ viewerNamed?: boolean; /** Someone other than the viewer or its agent created the chat. */ @@ -151,12 +148,10 @@ function fixture({ }, ); // The default agent comes first, so a named agent is distinguishable from it. - const agents = agentless - ? [] - : [ - { pubkey: "b".repeat(64), name: "Helper", managed: false }, - { pubkey: agent, name: "Scout", managed: false }, - ]; + const agents = [ + { pubkey: "b".repeat(64), name: "Helper", managed: false }, + { pubkey: agent, name: "Scout", managed: false }, + ]; const choices = { ...owner.session.agentChoices.snapshot(), status: "ready" as const, @@ -659,24 +654,9 @@ it("keeps a saved draft that only tags the default agent in Back history", async expect(readView(f.scope, "solo:new-draft", null)).toEqual(typed); }); -it("keeps a linked chat draft in Back history, since it holds its source chat", async () => { +it("keeps a linked chat draft in Back history until its source is used", async () => { const linked = "22222222-2222-4222-8222-222222222222"; const f = fixture({ target: (scope) => soloDraftTarget(scope, { linked }) }); - writeView(f.scope, linkedDraftKey(linked), { - channelId: "33333333-3333-4333-8333-333333333333", - link: `buzz://message?channel=33333333-3333-4333-8333-333333333333&id=${"e".repeat(64)}`, - }); - f.mount(); - await screen.findByRole("textbox"); - expect(vi.mocked(f.navigation.passThrough).mock.lastCall?.[0]()).toBe(false); -}); - -it("keeps an untagged linked chat draft in Back history until its source is used", async () => { - const linked = "22222222-2222-4222-8222-222222222222"; - const f = fixture({ - agentless: true, - target: (scope) => soloDraftTarget(scope, { linked }), - }); const link = `buzz://message?channel=33333333-3333-4333-8333-333333333333&id=${"e".repeat(64)}`; writeView(f.scope, linkedDraftKey(linked), { channelId: "33333333-3333-4333-8333-333333333333", diff --git a/src/features/direct-messages/NewMessage.recovery.test.tsx b/src/features/direct-messages/NewMessage.recovery.test.tsx index 04704d5a92..5a06737ce9 100644 --- a/src/features/direct-messages/NewMessage.recovery.test.tsx +++ b/src/features/direct-messages/NewMessage.recovery.test.tsx @@ -1106,8 +1106,12 @@ it("confirms a photo-only uncertain send after session restart without files or const original = t.publish.mock.calls[0]?.[0]; f.view.unmount(); f.owner.dispose(); - t.mount(t.create()); - await t.user.click(await screen.findByRole("button", { name: "Retry send" })); + const passThrough = vi.fn(() => () => {}); + t.mount(t.create(), undefined, { passThrough } as unknown as PageNavigation); + const retry = await screen.findByRole("button", { name: "Retry send" }); + // The held photo keeps the retry page in Back history. + expect(passThrough.mock.lastCall?.[0]()).toBe(false); + await t.user.click(retry); await waitFor(() => expect(t.onStarted).toHaveBeenCalledOnce()); expect(t.publish.mock.calls[1]?.[0]).toEqual(original); }); diff --git a/src/features/direct-messages/NewMessage.tsx b/src/features/direct-messages/NewMessage.tsx index f177164d45..5d1adf1e2d 100644 --- a/src/features/direct-messages/NewMessage.tsx +++ b/src/features/direct-messages/NewMessage.tsx @@ -155,16 +155,16 @@ export function NewMessage({ ): void; onStarted(channelId: string, messageId: string): void; }) { - // A message held for delivery or retry is shown even when its draft view is - // gone, so it counts as drafted too. - const held = useRef(undefined); - const heldDraft = useCallback(() => held.current, []); + // A message held for delivery or retry stays a Back stop even when its + // draft view is gone or it carries only attachments. + const held = useRef(false); + const holds = useCallback(() => held.current, []); const draftEdited = usePassThroughUntilDrafted( visit, session, scope, draftKey, - heldDraft, + { held: holds }, ); const [recipients, setRecipients] = useState(() => savedRecipients(scope, session.viewer), @@ -204,7 +204,7 @@ export function NewMessage({ } catch { /* Readiness presents invalid recovery. */ } - held.current = pending?.draft; + held.current = !!pending; const failed = pending && session.directMessages.delivery(pending.id) === "failed"; const locked = !ready || !!busy || (!!pending && !failed); diff --git a/src/features/messages/draft-visit.ts b/src/features/messages/draft-visit.ts index a61298cdd3..a7d91ae82c 100644 --- a/src/features/messages/draft-visit.ts +++ b/src/features/messages/draft-visit.ts @@ -11,15 +11,22 @@ import { /** A new conversation's visit is a Back stop only once its composer draft * holds something the person wrote; until then the next page replaces it. - * `initialDraft` reads what the composer shows while nothing is saved. - * Returns the composer's edit listener; the page passes `undefined` once it - * has spent the draft. */ + * `initialDraft` reads what the composer shows while nothing is saved; + * `held` reports a message the page holds for delivery or retry, a Back stop + * whatever it contains. Returns the composer's edit listener; the page passes + * `undefined` once it has spent the draft. */ export function usePassThroughUntilDrafted( visit: PageNavigation | undefined, session: ComposerSession, scope: string, draftKey: string, - initialDraft?: () => MentionDraft | string | undefined, + { + initialDraft, + held, + }: { + initialDraft?: () => MentionDraft | string | undefined; + held?: () => boolean; + } = {}, ) { // The composer's latest edit stands for the saved draft, which may be // missing when storage fails, until the page spends it. @@ -28,6 +35,7 @@ export function usePassThroughUntilDrafted( () => visit?.passThrough( () => + !held?.() && !hasDraftContent( edited.current ?? mentionDraft( @@ -36,7 +44,7 @@ export function usePassThroughUntilDrafted( attachmentDraftCount(session, `${scope}:${draftKey}`), ), ), - [visit, session, scope, draftKey, initialDraft], + [visit, session, scope, draftKey, initialDraft, held], ); return useCallback((draft: MentionDraft | undefined) => { edited.current = draft; diff --git a/src/features/solo/SoloDraft.tsx b/src/features/solo/SoloDraft.tsx index bd70dd7242..dcec657d0e 100644 --- a/src/features/solo/SoloDraft.tsx +++ b/src/features/solo/SoloDraft.tsx @@ -135,7 +135,7 @@ export function SoloDraft({ session, scope, `${draftKey}:new-draft`, - linkedSource, + { initialDraft: linkedSource }, ); const [settingsOpen, setSettingsOpen] = useState(false); // New chats tag their agent in the draft: the one the route names, else the