From c710868fbca9bdf7adb155c68ecbdd74858ede9f Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Wed, 10 Jun 2026 23:39:35 -0500 Subject: [PATCH] refactor(desktop): decouple composer from session lifecycle entirely MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The composer is a single global surface that sits ABOVE the thread: its contents follow the user across session switches and are never touched by session lifecycle. Switching threads doesn't change the render. Replaces the per-scope draft choreography (scoped storage keys, attachment stash map, skip-sentinel, restore-on-scope-change effect) with: - one global localStorage key so an unsent draft survives app reloads - a one-shot restore on mount - nothing else — session switches simply don't touch the composer Verified E2E via CDP with real sidebar clicks + real keystrokes: typed draft survives A->B->A switching and a full page reload. --- apps/desktop/src/app/chat/composer/index.tsx | 78 +++++++------------ apps/desktop/src/store/composer.test.ts | 82 ++++---------------- apps/desktop/src/store/composer.ts | 64 +++------------ 3 files changed, 50 insertions(+), 174 deletions(-) diff --git a/apps/desktop/src/app/chat/composer/index.tsx b/apps/desktop/src/app/chat/composer/index.tsx index e55e5019205..511713d086f 100644 --- a/apps/desktop/src/app/chat/composer/index.tsx +++ b/apps/desktop/src/app/chat/composer/index.tsx @@ -28,11 +28,8 @@ import { $composerAttachments, clearComposerAttachments, clearPersistedComposerDraft, - clearStashedComposerAttachments, type ComposerAttachment, readPersistedComposerDraft, - stashComposerAttachments, - takeComposerAttachments, writePersistedComposerDraft } from '@/store/composer' import { @@ -174,7 +171,6 @@ export function ChatBar({ const scrolledUp = useStore($threadScrolledUp) const sessionMessages = useStore($messages) const activeQueueSessionKey = queueSessionKey || sessionId || null - const draftPersistenceScope = activeQueueSessionKey || null const queuedPrompts = useMemo( () => (activeQueueSessionKey ? (queuedPromptsBySession[activeQueueSessionKey] ?? []) : []), @@ -186,12 +182,7 @@ export function ChatBar({ const editorRef = useRef(null) const draftRef = useRef(draft) const previousBusyRef = useRef(busy) - // `undefined` = no skip pending. The sentinel must be distinguishable from a - // real scope, and `null` IS a real scope (the unsaved-new-session draft): - // resetting to null made every persist run in a new chat match the consumed - // sentinel and bail, so new-chat drafts were never written at all. - const skipNextDraftPersistScopeRef = useRef(undefined) - const pendingDraftPersistRef = useRef<{ scope: string | null; value: string } | null>(null) + const pendingDraftPersistRef = useRef(null) const drainingQueueRef = useRef(false) const urlInputRef = useRef(null) @@ -1118,65 +1109,50 @@ export function ChatBar({ } } - // Restore a scope's draft (persisted text + in-memory attachments) when we - // enter it, and stash the attachments back when we leave. Text rides through - // localStorage so it survives reloads; attachments carry live blobs/upload - // state that can't serialize, so they're retained in memory only — enough to - // survive a session switch, which is the case users actually hit. + // The composer deliberately does NOT react to session switches: it sits + // above the thread and its contents follow the user. The only restore is a + // one-shot on mount so an unsent draft survives an app reload. useEffect(() => { - const persisted = readPersistedComposerDraft(draftPersistenceScope) - const restoredAttachments = takeComposerAttachments(draftPersistenceScope) - skipNextDraftPersistScopeRef.current = draftPersistenceScope - loadIntoComposer(persisted, restoredAttachments) + const persisted = readPersistedComposerDraft() - return () => { - stashComposerAttachments(draftPersistenceScope, $composerAttachments.get()) + if (persisted && !draftRef.current.trim()) { + loadIntoComposer(persisted, $composerAttachments.get()) } - }, [draftPersistenceScope]) // eslint-disable-line react-hooks/exhaustive-deps + }, []) // eslint-disable-line react-hooks/exhaustive-deps useEffect(() => { - if (skipNextDraftPersistScopeRef.current === draftPersistenceScope) { - skipNextDraftPersistScopeRef.current = undefined - - return - } - // Don't persist programmatically-loaded text: browsing sent-message // history or editing a queued prompt swaps the composer to recalled text, // and persisting that would clobber the genuine in-progress draft (which - // history keeps in its own snapshot and restores on the way back). Leaving - // the prior pending write untouched keeps the real draft in storage. + // history keeps in its own snapshot and restores on the way back). if (isBrowsingHistory(sessionId) || queueEdit) { return } // Debounce the localStorage write: the composer's per-keystroke path was // deliberately slimmed down (see the draftRef sync comment above), so we - // don't touch storage on every keypress. The pending ref below is flushed - // on scope change / unmount so a fast session switch can't drop the - // trailing keystrokes. - pendingDraftPersistRef.current = { scope: draftPersistenceScope, value: draft } + // don't touch storage on every keypress. + pendingDraftPersistRef.current = draft const handle = window.setTimeout(() => { pendingDraftPersistRef.current = null - writePersistedComposerDraft(draftPersistenceScope, draft) + writePersistedComposerDraft(draft) }, DRAFT_PERSIST_DEBOUNCE_MS) return () => window.clearTimeout(handle) - }, [draft, draftPersistenceScope, queueEdit, sessionId]) + }, [draft, queueEdit, sessionId]) - // Flush any pending debounced draft write when leaving a session scope, - // unmounting, or the window unloading, so the latest text is always - // persisted. The pagehide listener is load-bearing: React does NOT run - // effect cleanups on a page reload, so without it a Cmd+R inside the - // debounce window silently dropped everything typed in the last 400ms. + // Flush any pending debounced write on unmount or window unload. The + // pagehide listener is load-bearing: React does NOT run effect cleanups on + // a page reload, so without it a Cmd+R inside the debounce window would + // silently drop everything typed in the last 400ms. useEffect(() => { const flushPendingDraftPersist = () => { const pending = pendingDraftPersistRef.current - if (pending) { + if (pending !== null) { pendingDraftPersistRef.current = null - writePersistedComposerDraft(pending.scope, pending.value) + writePersistedComposerDraft(pending) } } @@ -1186,7 +1162,7 @@ export function ChatBar({ window.removeEventListener('pagehide', flushPendingDraftPersist) flushPendingDraftPersist() } - }, [draftPersistenceScope]) + }, []) const beginQueuedEdit = (entry: QueuedPromptEntry) => { if (!activeQueueSessionKey || queueEdit) { @@ -1444,14 +1420,13 @@ export function ChatBar({ void Promise.resolve(onSubmit(submitted)).then(accepted => { if (accepted === false) { loadIntoComposer(submitted, []) - writePersistedComposerDraft(draftPersistenceScope, submitted) + writePersistedComposerDraft(submitted) } else { - clearPersistedComposerDraft(draftPersistenceScope) - clearStashedComposerAttachments(draftPersistenceScope) + clearPersistedComposerDraft() } }).catch(() => { loadIntoComposer(submitted, []) - writePersistedComposerDraft(draftPersistenceScope, submitted) + writePersistedComposerDraft(submitted) }) } else if (payloadPresent) { queueCurrentDraft() @@ -1473,14 +1448,13 @@ export function ChatBar({ void Promise.resolve(onSubmit(submitted, { attachments: submittedAttachments })).then(accepted => { if (accepted === false) { loadIntoComposer(submitted, submittedAttachments) - writePersistedComposerDraft(draftPersistenceScope, submitted) + writePersistedComposerDraft(submitted) } else { - clearPersistedComposerDraft(draftPersistenceScope) - clearStashedComposerAttachments(draftPersistenceScope) + clearPersistedComposerDraft() } }).catch(() => { loadIntoComposer(submitted, submittedAttachments) - writePersistedComposerDraft(draftPersistenceScope, submitted) + writePersistedComposerDraft(submitted) }) } diff --git a/apps/desktop/src/store/composer.test.ts b/apps/desktop/src/store/composer.test.ts index 05ecef22558..efa9fcba4db 100644 --- a/apps/desktop/src/store/composer.test.ts +++ b/apps/desktop/src/store/composer.test.ts @@ -4,13 +4,10 @@ import { $composerAttachments, addComposerAttachment, clearPersistedComposerDraft, - clearStashedComposerAttachments, + COMPOSER_DRAFT_STORAGE_KEY, type ComposerAttachment, - composerDraftStorageKey, readPersistedComposerDraft, removeComposerAttachment, - stashComposerAttachments, - takeComposerAttachments, updateComposerAttachment, writePersistedComposerDraft } from './composer' @@ -49,83 +46,30 @@ describe('updateComposerAttachment', () => { }) }) -describe('persisted composer drafts', () => { +describe('persisted composer draft', () => { afterEach(() => { window.localStorage.clear() }) - it('stores and restores text drafts per session scope', () => { - writePersistedComposerDraft('session-a', 'almost submitted prompt') - writePersistedComposerDraft('session-b', 'other draft') + it('stores and restores the draft', () => { + writePersistedComposerDraft('almost submitted prompt') - expect(readPersistedComposerDraft('session-a')).toBe('almost submitted prompt') - expect(readPersistedComposerDraft('session-b')).toBe('other draft') - }) - - it('uses a stable new-session key when no session id exists yet', () => { - writePersistedComposerDraft(null, 'first prompt draft') - - expect(window.localStorage.getItem(composerDraftStorageKey(null))).toBe('first prompt draft') - expect(readPersistedComposerDraft(undefined)).toBe('first prompt draft') + expect(readPersistedComposerDraft()).toBe('almost submitted prompt') + expect(window.localStorage.getItem(COMPOSER_DRAFT_STORAGE_KEY)).toBe('almost submitted prompt') }) it('removes empty drafts instead of leaving stale text behind', () => { - writePersistedComposerDraft('session-a', 'saved') - writePersistedComposerDraft('session-a', '') + writePersistedComposerDraft('saved') + writePersistedComposerDraft('') - expect(readPersistedComposerDraft('session-a')).toBe('') - expect(window.localStorage.getItem(composerDraftStorageKey('session-a'))).toBeNull() + expect(readPersistedComposerDraft()).toBe('') + expect(window.localStorage.getItem(COMPOSER_DRAFT_STORAGE_KEY)).toBeNull() }) it('can explicitly clear a saved draft after submit', () => { - writePersistedComposerDraft('session-a', 'saved') - clearPersistedComposerDraft('session-a') + writePersistedComposerDraft('saved') + clearPersistedComposerDraft() - expect(readPersistedComposerDraft('session-a')).toBe('') - }) -}) - -describe('stashed composer attachments', () => { - afterEach(() => { - clearStashedComposerAttachments('session-a') - clearStashedComposerAttachments('session-b') - clearStashedComposerAttachments(null) - }) - - it('retains and restores attachments per session scope', () => { - stashComposerAttachments('session-a', [attachment({ id: 'file:a' })]) - stashComposerAttachments('session-b', [attachment({ id: 'image:b', kind: 'image' })]) - - expect(takeComposerAttachments('session-a').map(a => a.id)).toEqual(['file:a']) - expect(takeComposerAttachments('session-b').map(a => a.id)).toEqual(['image:b']) - }) - - it('shares a stable new-session scope with the text draft helpers', () => { - stashComposerAttachments(null, [attachment({ id: 'file:new' })]) - - expect(takeComposerAttachments(undefined).map(a => a.id)).toEqual(['file:new']) - }) - - it('returns cloned attachments so callers cannot mutate the stash', () => { - stashComposerAttachments('session-a', [attachment({ id: 'file:a', label: 'orig.pdf' })]) - - const taken = takeComposerAttachments('session-a') - taken[0]!.label = 'mutated.pdf' - - expect(takeComposerAttachments('session-a')[0]?.label).toBe('orig.pdf') - }) - - it('drops the scope entry when stashing an empty set', () => { - stashComposerAttachments('session-a', [attachment({ id: 'file:a' })]) - stashComposerAttachments('session-a', []) - - expect(takeComposerAttachments('session-a')).toEqual([]) - }) - - it('clears a scope explicitly after an accepted submit', () => { - stashComposerAttachments('session-a', [attachment({ id: 'file:a' })]) - clearStashedComposerAttachments('session-a') - - expect(takeComposerAttachments('session-a')).toEqual([]) + expect(readPersistedComposerDraft()).toBe('') }) }) diff --git a/apps/desktop/src/store/composer.ts b/apps/desktop/src/store/composer.ts index ba78ac43e22..35d372521cc 100644 --- a/apps/desktop/src/store/composer.ts +++ b/apps/desktop/src/store/composer.ts @@ -21,14 +21,10 @@ export const $composerDraft = atom('') export const $composerAttachments = atom([]) export const $composerTerminalSelections = atom>({}) -const COMPOSER_DRAFT_STORAGE_PREFIX = 'hermes:composer-draft:v1:' -const NEW_SESSION_DRAFT_SCOPE = '__new__' - -function storageScope(scope: string | null | undefined): string { - const trimmed = scope?.trim() - - return trimmed || NEW_SESSION_DRAFT_SCOPE -} +// The composer is a single global surface that sits ABOVE the thread: its +// contents follow the user across session switches and are never touched by +// session lifecycle. One storage key makes the draft survive app reloads. +export const COMPOSER_DRAFT_STORAGE_KEY = 'hermes:composer-draft:v2' function browserStorage(): Storage | null { if (typeof window === 'undefined') { @@ -42,19 +38,15 @@ function browserStorage(): Storage | null { } } -export function composerDraftStorageKey(scope: string | null | undefined): string { - return `${COMPOSER_DRAFT_STORAGE_PREFIX}${encodeURIComponent(storageScope(scope))}` -} - -export function readPersistedComposerDraft(scope: string | null | undefined): string { +export function readPersistedComposerDraft(): string { try { - return browserStorage()?.getItem(composerDraftStorageKey(scope)) ?? '' + return browserStorage()?.getItem(COMPOSER_DRAFT_STORAGE_KEY) ?? '' } catch { return '' } } -export function writePersistedComposerDraft(scope: string | null | undefined, value: string) { +export function writePersistedComposerDraft(value: string) { try { const storage = browserStorage() @@ -62,12 +54,10 @@ export function writePersistedComposerDraft(scope: string | null | undefined, va return } - const key = composerDraftStorageKey(scope) - if (value.length === 0) { - storage.removeItem(key) + storage.removeItem(COMPOSER_DRAFT_STORAGE_KEY) } else { - storage.setItem(key, value) + storage.setItem(COMPOSER_DRAFT_STORAGE_KEY, value) } } catch { // Draft persistence is a safety net only; storage quota/private-mode errors @@ -75,46 +65,14 @@ export function writePersistedComposerDraft(scope: string | null | undefined, va } } -export function clearPersistedComposerDraft(scope: string | null | undefined) { +export function clearPersistedComposerDraft() { try { - browserStorage()?.removeItem(composerDraftStorageKey(scope)) + browserStorage()?.removeItem(COMPOSER_DRAFT_STORAGE_KEY) } catch { // Best-effort only. } } -// Attachments can't ride along in localStorage the way text does — they carry -// live blobs, object URLs, and in-flight upload state that don't serialize and -// are tied to the running app. So we retain them per scope in an in-memory map -// instead: a session switch restores the chips you'd staged, even though they -// (unlike text) cannot survive a full app reload. -const composerAttachmentsByScope = new Map() - -const cloneComposerAttachments = (attachments: ComposerAttachment[]): ComposerAttachment[] => - attachments.map(attachment => ({ ...attachment })) - -export function stashComposerAttachments(scope: string | null | undefined, attachments: ComposerAttachment[]) { - const key = storageScope(scope) - - if (attachments.length === 0) { - composerAttachmentsByScope.delete(key) - - return - } - - composerAttachmentsByScope.set(key, cloneComposerAttachments(attachments)) -} - -export function takeComposerAttachments(scope: string | null | undefined): ComposerAttachment[] { - const stashed = composerAttachmentsByScope.get(storageScope(scope)) - - return stashed ? cloneComposerAttachments(stashed) : [] -} - -export function clearStashedComposerAttachments(scope: string | null | undefined) { - composerAttachmentsByScope.delete(storageScope(scope)) -} - export function setComposerDraft(value: string) { $composerDraft.set(value) }