diff --git a/web/src/components/AssistantChat/HappyComposer.tsx b/web/src/components/AssistantChat/HappyComposer.tsx index cab3e014..6d0e20d3 100644 --- a/web/src/components/AssistantChat/HappyComposer.tsx +++ b/web/src/components/AssistantChat/HappyComposer.tsx @@ -22,6 +22,7 @@ import { usePlatform } from '@/hooks/usePlatform' import { usePWAInstall } from '@/hooks/usePWAInstall' import { supportsEffort, supportsModelChange } from '@hapi/protocol' import { markSkillUsed } from '@/lib/recent-skills' +import { useComposerDraft } from '@/hooks/useComposerDraft' import { FloatingOverlay } from '@/components/ChatInput/FloatingOverlay' import { Autocomplete } from '@/components/ChatInput/Autocomplete' import { StatusBar } from '@/components/AssistantChat/StatusBar' @@ -40,6 +41,7 @@ export interface TextInputState { const defaultSuggestionHandler = async (): Promise => [] export function HappyComposer(props: { + sessionId?: string disabled?: boolean permissionMode?: PermissionMode collaborationMode?: CodexCollaborationMode @@ -72,6 +74,7 @@ export function HappyComposer(props: { }) { const { t } = useTranslation() const { + sessionId, disabled = false, permissionMode: rawPermissionMode, collaborationMode: rawCollaborationMode, @@ -143,6 +146,8 @@ export function HappyComposer(props: { const textareaRef = useRef(null) const prevControlledByUser = useRef(controlledByUser) + useComposerDraft(sessionId, composerText, (text) => api.composer().setText(text)) + useEffect(() => { setInputState((prev) => { if (prev.text === composerText) return prev diff --git a/web/src/components/SessionChat.tsx b/web/src/components/SessionChat.tsx index 1180aeb3..2a60c62b 100644 --- a/web/src/components/SessionChat.tsx +++ b/web/src/components/SessionChat.tsx @@ -388,6 +388,8 @@ export function SessionChat(props: { /> ({ + appendOptimisticMessage: vi.fn(), + getMessageWindowState: vi.fn(() => ({ messages: [], pending: [] })), + updateMessageStatus: vi.fn(), +})) + +vi.mock('@/hooks/usePlatform', () => ({ + usePlatform: () => ({ + haptic: { notification: vi.fn() }, + }), +})) + +vi.mock('@/lib/messages', () => ({ + makeClientSideId: vi.fn(() => 'local-id-1'), +})) + +function createWrapper() { + const queryClient = new QueryClient({ + defaultOptions: { mutations: { retry: false } }, + }) + return function Wrapper({ children }: { children: ReactNode }) { + return {children} + } +} + +function createMockApi(sendMessage: (...args: unknown[]) => Promise = async () => {}): ApiClient { + return { sendMessage } as unknown as ApiClient +} + +describe('useSendMessage', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('calls onSuccess with the session ID that was sent', async () => { + const onSuccess = vi.fn() + const api = createMockApi() + + const { result } = renderHook( + () => useSendMessage(api, 'session-A', { onSuccess }), + { wrapper: createWrapper() }, + ) + + act(() => { + result.current.sendMessage('hello') + }) + + await waitFor(() => { + expect(onSuccess).toHaveBeenCalledWith('session-A') + }) + }) + + it('calls onSuccess with resolved session ID, not the original', async () => { + const onSuccess = vi.fn() + const api = createMockApi() + + const { result } = renderHook( + () => useSendMessage(api, 'session-original', { + onSuccess, + resolveSessionId: async () => 'session-resolved', + onSessionResolved: vi.fn(), + }), + { wrapper: createWrapper() }, + ) + + act(() => { + result.current.sendMessage('hello') + }) + + await waitFor(() => { + expect(onSuccess).toHaveBeenCalledWith('session-resolved') + }) + }) + + it('does not call onSuccess when send fails', async () => { + const onSuccess = vi.fn() + const api = createMockApi(async () => { + throw new Error('network error') + }) + + const { result } = renderHook( + () => useSendMessage(api, 'session-A', { onSuccess }), + { wrapper: createWrapper() }, + ) + + act(() => { + result.current.sendMessage('hello') + }) + + await waitFor(() => { + expect(result.current.isSending).toBe(false) + }) + + expect(onSuccess).not.toHaveBeenCalled() + }) + + it('does not call onSuccess when blocked', () => { + const onSuccess = vi.fn() + const onBlocked = vi.fn() + + const { result } = renderHook( + () => useSendMessage(null, 'session-A', { onSuccess, onBlocked }), + { wrapper: createWrapper() }, + ) + + act(() => { + result.current.sendMessage('hello') + }) + + expect(onBlocked).toHaveBeenCalledWith('no-api') + expect(onSuccess).not.toHaveBeenCalled() + }) +}) diff --git a/web/src/hooks/mutations/useSendMessage.ts b/web/src/hooks/mutations/useSendMessage.ts index 6ca5ae63..6d120d9f 100644 --- a/web/src/hooks/mutations/useSendMessage.ts +++ b/web/src/hooks/mutations/useSendMessage.ts @@ -24,6 +24,7 @@ type UseSendMessageOptions = { resolveSessionId?: (sessionId: string) => Promise onSessionResolved?: (sessionId: string) => void onBlocked?: (reason: BlockedReason) => void + onSuccess?: (sessionId: string) => void } function findMessageByLocalId( @@ -83,6 +84,7 @@ export function useSendMessage( onSuccess: (_, input) => { updateMessageStatus(input.sessionId, input.localId, 'sent') haptic.notification('success') + options?.onSuccess?.(input.sessionId) }, onError: (_, input) => { updateMessageStatus(input.sessionId, input.localId, 'failed') diff --git a/web/src/hooks/useComposerDraft.test.ts b/web/src/hooks/useComposerDraft.test.ts new file mode 100644 index 00000000..e0eb6b04 --- /dev/null +++ b/web/src/hooks/useComposerDraft.test.ts @@ -0,0 +1,126 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { renderHook, act } from '@testing-library/react' + +// Mock composer-drafts module +vi.mock('@/lib/composer-drafts', () => ({ + getDraft: vi.fn(() => ''), + saveDraft: vi.fn(), +})) + +import { getDraft, saveDraft } from '@/lib/composer-drafts' +import { useComposerDraft } from './useComposerDraft' + +const mockGetDraft = vi.mocked(getDraft) +const mockSaveDraft = vi.mocked(saveDraft) + +describe('useComposerDraft', () => { + let rAFCallbacks: Array<() => void> + + beforeEach(() => { + vi.clearAllMocks() + rAFCallbacks = [] + vi.stubGlobal('requestAnimationFrame', vi.fn((cb: () => void) => { + rAFCallbacks.push(cb) + return rAFCallbacks.length + })) + vi.stubGlobal('cancelAnimationFrame', vi.fn()) + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + function flushRAF() { + const cbs = [...rAFCallbacks] + rAFCallbacks = [] + cbs.forEach(cb => cb()) + } + + it('restores saved draft on mount via requestAnimationFrame', () => { + mockGetDraft.mockReturnValue('saved text') + const setText = vi.fn() + + renderHook(() => useComposerDraft('session-1', '', setText)) + + // Before rAF fires, setText should not have been called + expect(setText).not.toHaveBeenCalled() + + // Flush rAF + act(() => flushRAF()) + + expect(mockGetDraft).toHaveBeenCalledWith('session-1') + expect(setText).toHaveBeenCalledWith('saved text') + }) + + it('does not restore draft if composer already has text', () => { + mockGetDraft.mockReturnValue('saved text') + const setText = vi.fn() + + renderHook(() => useComposerDraft('session-1', 'user is typing', setText)) + + act(() => flushRAF()) + + expect(setText).not.toHaveBeenCalled() + }) + + it('does not restore if draft is empty', () => { + mockGetDraft.mockReturnValue('') + const setText = vi.fn() + + renderHook(() => useComposerDraft('session-1', '', setText)) + + act(() => flushRAF()) + + expect(setText).not.toHaveBeenCalled() + }) + + it('saves draft on unmount after rAF has fired', () => { + mockGetDraft.mockReturnValue('') + const setText = vi.fn() + + const { unmount, rerender } = renderHook( + ({ text }) => useComposerDraft('session-1', text, setText), + { initialProps: { text: '' } }, + ) + + // Fire rAF to set draftReady = true + act(() => flushRAF()) + + // Simulate user typing + rerender({ text: 'my draft' }) + + unmount() + + expect(mockSaveDraft).toHaveBeenCalledWith('session-1', 'my draft') + }) + + it('does not save draft on unmount before rAF has fired', () => { + mockGetDraft.mockReturnValue('') + const setText = vi.fn() + + const { unmount } = renderHook( + () => useComposerDraft('session-1', 'some text', setText), + ) + + // Unmount before rAF fires (draftReady is still false) + unmount() + + expect(mockSaveDraft).not.toHaveBeenCalled() + expect(vi.mocked(cancelAnimationFrame)).toHaveBeenCalled() + }) + + it('does nothing when sessionId is undefined', () => { + const setText = vi.fn() + + const { unmount } = renderHook( + () => useComposerDraft(undefined, 'text', setText), + ) + + act(() => flushRAF()) + unmount() + + expect(mockGetDraft).not.toHaveBeenCalled() + expect(mockSaveDraft).not.toHaveBeenCalled() + expect(setText).not.toHaveBeenCalled() + }) +}) diff --git a/web/src/hooks/useComposerDraft.ts b/web/src/hooks/useComposerDraft.ts new file mode 100644 index 00000000..431a04c4 --- /dev/null +++ b/web/src/hooks/useComposerDraft.ts @@ -0,0 +1,41 @@ +import { useEffect, useRef } from 'react' +import { getDraft, saveDraft } from '@/lib/composer-drafts' + +/** + * Manages draft save/restore lifecycle for a composer. + * + * - On mount: restores saved draft via `setText` (deferred by one animation frame) + * - On unmount: saves current text as draft + * - The `draftReady` guard prevents saving before the initial restore completes, + * avoiding the case where the runtime's empty initial text overwrites a real draft. + */ +export function useComposerDraft( + sessionId: string | undefined, + composerText: string, + setText: (text: string) => void, +): void { + const composerTextRef = useRef(composerText) + composerTextRef.current = composerText + + const draftReadyRef = useRef(false) + + useEffect(() => { + if (!sessionId) return + + const frame = requestAnimationFrame(() => { + const draft = getDraft(sessionId) + if (draft && !composerTextRef.current) { + setText(draft) + } + draftReadyRef.current = true + }) + + return () => { + cancelAnimationFrame(frame) + if (draftReadyRef.current) { + saveDraft(sessionId, composerTextRef.current) + } + draftReadyRef.current = false + } + }, [sessionId]) // eslint-disable-line react-hooks/exhaustive-deps +} diff --git a/web/src/lib/clearDraftsAfterSend.test.ts b/web/src/lib/clearDraftsAfterSend.test.ts new file mode 100644 index 00000000..6d1fd0dc --- /dev/null +++ b/web/src/lib/clearDraftsAfterSend.test.ts @@ -0,0 +1,35 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' + +vi.mock('@/lib/composer-drafts', () => ({ + clearDraft: vi.fn(), +})) + +import { clearDraft } from '@/lib/composer-drafts' +import { clearDraftsAfterSend } from './clearDraftsAfterSend' + +const mockClearDraft = vi.mocked(clearDraft) + +describe('clearDraftsAfterSend', () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it('clears the sent session draft', () => { + clearDraftsAfterSend('session-A', 'session-A') + expect(mockClearDraft).toHaveBeenCalledWith('session-A') + expect(mockClearDraft).toHaveBeenCalledTimes(1) + }) + + it('clears both drafts when session was resolved to a different ID', () => { + clearDraftsAfterSend('resolved-B', 'session-A') + expect(mockClearDraft).toHaveBeenCalledWith('resolved-B') + expect(mockClearDraft).toHaveBeenCalledWith('session-A') + expect(mockClearDraft).toHaveBeenCalledTimes(2) + }) + + it('only clears sent session when route session is null', () => { + clearDraftsAfterSend('session-A', null) + expect(mockClearDraft).toHaveBeenCalledWith('session-A') + expect(mockClearDraft).toHaveBeenCalledTimes(1) + }) +}) diff --git a/web/src/lib/clearDraftsAfterSend.ts b/web/src/lib/clearDraftsAfterSend.ts new file mode 100644 index 00000000..c35c057d --- /dev/null +++ b/web/src/lib/clearDraftsAfterSend.ts @@ -0,0 +1,16 @@ +import { clearDraft } from '@/lib/composer-drafts' + +/** + * Clear draft(s) after a successful send. + * When `resolveSessionId` swaps the session (e.g. inactive → resumed), + * the sent ID differs from the route's session ID, so both must be cleared. + */ +export function clearDraftsAfterSend( + sentSessionId: string, + routeSessionId: string | null, +): void { + clearDraft(sentSessionId) + if (routeSessionId && sentSessionId !== routeSessionId) { + clearDraft(routeSessionId) + } +} diff --git a/web/src/lib/composer-drafts.test.ts b/web/src/lib/composer-drafts.test.ts new file mode 100644 index 00000000..bf26df80 --- /dev/null +++ b/web/src/lib/composer-drafts.test.ts @@ -0,0 +1,136 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +describe('composer-drafts', () => { + let storage: Record + + beforeEach(() => { + storage = {} + vi.stubGlobal('sessionStorage', { + getItem: vi.fn((key: string) => storage[key] ?? null), + setItem: vi.fn((key: string, value: string) => { storage[key] = value }), + removeItem: vi.fn((key: string) => { delete storage[key] }), + }) + // Force re-hydration by clearing the module's internal cache + // Re-import to reset the lazy-loaded cache + vi.resetModules() + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('returns empty string for unknown session', async () => { + const mod = await import('./composer-drafts') + expect(mod.getDraft('unknown-session')).toBe('') + }) + + it('saves and retrieves a draft', async () => { + const mod = await import('./composer-drafts') + mod.saveDraft('session-1', 'hello world') + expect(mod.getDraft('session-1')).toBe('hello world') + }) + + it('persists drafts to sessionStorage', async () => { + const mod = await import('./composer-drafts') + mod.saveDraft('session-1', 'test') + const stored = JSON.parse(storage['hapi:composer-drafts'] ?? '{}') + expect(stored['session-1']).toBe('test') + }) + + it('clears a draft', async () => { + const mod = await import('./composer-drafts') + mod.saveDraft('session-1', 'hello') + mod.clearDraft('session-1') + expect(mod.getDraft('session-1')).toBe('') + }) + + it('deletes entry when saving empty or whitespace-only text', async () => { + const mod = await import('./composer-drafts') + mod.saveDraft('session-1', 'hello') + expect(mod.getDraft('session-1')).toBe('hello') + + mod.saveDraft('session-1', ' ') + expect(mod.getDraft('session-1')).toBe('') + + const stored = JSON.parse(storage['hapi:composer-drafts'] ?? '{}') + expect(stored).not.toHaveProperty('session-1') + }) + + it('preserves untrimmed text when saving non-empty draft', async () => { + const mod = await import('./composer-drafts') + mod.saveDraft('session-1', ' hello ') + expect(mod.getDraft('session-1')).toBe(' hello ') + }) + + it('handles multiple sessions independently', async () => { + const mod = await import('./composer-drafts') + mod.saveDraft('session-a', 'text A') + mod.saveDraft('session-b', 'text B') + + expect(mod.getDraft('session-a')).toBe('text A') + expect(mod.getDraft('session-b')).toBe('text B') + + mod.clearDraft('session-a') + expect(mod.getDraft('session-a')).toBe('') + expect(mod.getDraft('session-b')).toBe('text B') + }) + + it('hydrates from existing sessionStorage data', async () => { + storage['hapi:composer-drafts'] = JSON.stringify({ 'existing': 'draft text' }) + const mod = await import('./composer-drafts') + expect(mod.getDraft('existing')).toBe('draft text') + }) + + it('recovers from invalid sessionStorage data', async () => { + storage['hapi:composer-drafts'] = 'not valid json' + const mod = await import('./composer-drafts') + expect(mod.getDraft('any')).toBe('') + // Should still be able to save + mod.saveDraft('any', 'recovered') + expect(mod.getDraft('any')).toBe('recovered') + }) + + it('ignores non-string values during hydration', async () => { + storage['hapi:composer-drafts'] = JSON.stringify({ + 'valid': 'text', + 'invalid-number': 42, + 'invalid-null': null, + }) + const mod = await import('./composer-drafts') + expect(mod.getDraft('valid')).toBe('text') + expect(mod.getDraft('invalid-number')).toBe('') + expect(mod.getDraft('invalid-null')).toBe('') + }) + + it('refreshes eviction order when updating an existing draft', async () => { + const mod = await import('./composer-drafts') + // Save 50 drafts (at capacity) + for (let i = 0; i < 50; i++) { + mod.saveDraft(`session-${i}`, `text-${i}`) + } + // Update the oldest one (session-0) — should move to end of eviction queue + mod.saveDraft('session-0', 'updated') + // Add one more to trigger eviction + mod.saveDraft('session-50', 'new') + // session-1 (the new oldest) should be evicted, not session-0 + expect(mod.getDraft('session-0')).toBe('updated') + expect(mod.getDraft('session-1')).toBe('') + expect(mod.getDraft('session-50')).toBe('new') + }) + + it('evicts oldest entries when exceeding MAX_DRAFTS', async () => { + const mod = await import('./composer-drafts') + // Save 55 drafts (MAX_DRAFTS is 50) + for (let i = 0; i < 55; i++) { + mod.saveDraft(`session-${i}`, `text-${i}`) + } + // Oldest 5 should be evicted + for (let i = 0; i < 5; i++) { + expect(mod.getDraft(`session-${i}`)).toBe('') + } + // Remaining 50 should still exist + for (let i = 5; i < 55; i++) { + expect(mod.getDraft(`session-${i}`)).toBe(`text-${i}`) + } + }) +}) diff --git a/web/src/lib/composer-drafts.ts b/web/src/lib/composer-drafts.ts new file mode 100644 index 00000000..b726c27a --- /dev/null +++ b/web/src/lib/composer-drafts.ts @@ -0,0 +1,91 @@ +const STORAGE_KEY = 'hapi:composer-drafts' +const MAX_DRAFTS = 50 + +type DraftsMap = Record + +let cache: DraftsMap | null = null + +function safeParseJson(value: string): unknown { + try { + return JSON.parse(value) as unknown + } catch { + return null + } +} + +function hydrate(): DraftsMap { + if (cache) return cache + if (typeof window === 'undefined') { + cache = {} + return cache + } + try { + const raw = sessionStorage.getItem(STORAGE_KEY) + if (!raw) { + cache = {} + return cache + } + const parsed = safeParseJson(raw) + if (!parsed || typeof parsed !== 'object') { + cache = {} + return cache + } + const record = parsed as Record + const result: DraftsMap = {} + for (const [key, value] of Object.entries(record)) { + if (key.trim().length === 0) continue + if (typeof value !== 'string') continue + result[key] = value + } + cache = result + return cache + } catch { + cache = {} + return cache + } +} + +function evict(drafts: DraftsMap): void { + const keys = Object.keys(drafts) + if (keys.length <= MAX_DRAFTS) return + // Remove oldest entries (first inserted) to stay under the cap + const excess = keys.length - MAX_DRAFTS + for (let i = 0; i < excess; i++) { + delete drafts[keys[i]!] + } +} + +function persist(): void { + if (typeof window === 'undefined') return + try { + const drafts = hydrate() + evict(drafts) + sessionStorage.setItem(STORAGE_KEY, JSON.stringify(drafts)) + } catch { + // Ignore storage errors + } +} + +export function getDraft(sessionId: string): string { + const drafts = hydrate() + return drafts[sessionId] ?? '' +} + +export function saveDraft(sessionId: string, text: string): void { + const trimmed = text.trim() + const drafts = hydrate() + if (!trimmed) { + delete drafts[sessionId] + } else { + // Delete before re-inserting to refresh Object.keys() order for eviction + delete drafts[sessionId] + drafts[sessionId] = text + } + persist() +} + +export function clearDraft(sessionId: string): void { + const drafts = hydrate() + delete drafts[sessionId] + persist() +} diff --git a/web/src/router.tsx b/web/src/router.tsx index 8cc05f05..2680d268 100644 --- a/web/src/router.tsx +++ b/web/src/router.tsx @@ -31,6 +31,7 @@ import { queryKeys } from '@/lib/query-keys' import { useToast } from '@/lib/toast-context' import { useTranslation } from '@/lib/use-translation' import { fetchLatestMessages, seedMessageWindowFromSession } from '@/lib/message-window-store' +import { clearDraftsAfterSend } from '@/lib/clearDraftsAfterSend' import type { Machine } from '@/types/api' import FilesPage from '@/routes/sessions/files' import FilePage from '@/routes/sessions/file' @@ -237,6 +238,9 @@ function SessionPage() { retryMessage, isSending, } = useSendMessage(api, sessionId, { + onSuccess: (sentSessionId) => { + clearDraftsAfterSend(sentSessionId, sessionId) + }, resolveSessionId: async (currentSessionId) => { if (!api || !session || session.active) { return currentSessionId