diff --git a/frontend/src/components/navigation/MobileTabBar.test.tsx b/frontend/src/components/navigation/MobileTabBar.test.tsx index 79403a57..fd06fb8b 100644 --- a/frontend/src/components/navigation/MobileTabBar.test.tsx +++ b/frontend/src/components/navigation/MobileTabBar.test.tsx @@ -77,7 +77,7 @@ describe('MobileTabBar', () => { expect(screen.getByText('Schedules')).toBeInTheDocument() }) - it('navigates to /assistant when assistant is clicked from repo context', async () => { + it('navigates to assistant session list when assistant is clicked from repo context', async () => { vi.mocked(useMobile).mockReturnValue(true) const queryClient = new QueryClient() const user = userEvent.setup() @@ -96,10 +96,10 @@ describe('MobileTabBar', () => { ) await user.click(screen.getByRole('button', { name: 'Assistant' })) - expect(screen.getByTestId('location')).toHaveTextContent('/assistant') + expect(screen.getByTestId('location')).toHaveTextContent('/assistant?view=sessions') }) - it('navigates to assistant route when assistant is clicked without repo id', async () => { + it('navigates to assistant session list when assistant is clicked without repo id', async () => { vi.mocked(useMobile).mockReturnValue(true) const queryClient = new QueryClient() const user = userEvent.setup() @@ -118,7 +118,7 @@ describe('MobileTabBar', () => { ) await user.click(screen.getByRole('button', { name: 'Assistant' })) - expect(screen.getByTestId('location')).toHaveTextContent('/assistant') + expect(screen.getByTestId('location')).toHaveTextContent('/assistant?view=sessions') }) it('renders schedule tabs on /repos/:id/schedules path', () => { diff --git a/frontend/src/components/navigation/MobileTabBar.tsx b/frontend/src/components/navigation/MobileTabBar.tsx index c20880bc..c4b0b80b 100644 --- a/frontend/src/components/navigation/MobileTabBar.tsx +++ b/frontend/src/components/navigation/MobileTabBar.tsx @@ -6,7 +6,7 @@ import { useMobile } from '@/hooks/useMobile' import { useMobileTabBar } from '@/hooks/useMobileTabBar' import { useUrlParams } from '@/hooks/useUrlParams' import { useScheduleUrlState, type ScheduleTab } from '@/hooks/useScheduleUrlState' -import { getAssistantPath, isAssistantPath } from '@/lib/navigation' +import { getAssistantPath, getAssistantSessionListPath, isAssistantPath } from '@/lib/navigation' interface TabDef { key: string @@ -74,7 +74,7 @@ function buildGlobalTabs({ pathname, openSheet, open, close, navigate, isInsideR const handleAssistantClick = () => { close() - navigate(getAssistantPath()) + navigate(getAssistantSessionListPath()) } return [ @@ -97,7 +97,7 @@ function buildGlobalTabs({ pathname, openSheet, open, close, navigate, isInsideR label: 'Assistant', icon: Bot, onClick: handleAssistantClick, - active: isAssistantPath(pathname) && !openSheet, + active: (pathname === getAssistantPath() || isAssistantPath(pathname)) && !openSheet, }, { key: 'schedules', diff --git a/frontend/src/components/navigation/moreDrawerItems.test.ts b/frontend/src/components/navigation/moreDrawerItems.test.ts index 6ea63961..bfb6283a 100644 --- a/frontend/src/components/navigation/moreDrawerItems.test.ts +++ b/frontend/src/components/navigation/moreDrawerItems.test.ts @@ -90,7 +90,7 @@ describe('buildNavModel', () => { expect(model.primary[0].key).toBe('new-repo') expect(model.primary[0].onSelect).toBe('new-repo') expect(model.primary[1].key).toBe('assistant') - expect(model.primary[1].to).toBe('/assistant') + expect(model.primary[1].to).toBe('/assistant?view=sessions') }) it('returns new-session and assistant primary CTAs for repo detail', () => { @@ -99,7 +99,7 @@ describe('buildNavModel', () => { expect(model.primary[0].key).toBe('new-session') expect(model.primary[0].onSelect).toBe('new-session') expect(model.primary[1].key).toBe('assistant') - expect(model.primary[1].to).toBe('/assistant') + expect(model.primary[1].to).toBe('/assistant?view=sessions') }) it('returns new-session and assistant primary CTAs for session detail', () => { @@ -109,7 +109,7 @@ describe('buildNavModel', () => { expect(model.primary[0].onSelect).toBe('new-session') expect(model.primary[0].variant).toBe('primary') expect(model.primary[1].key).toBe('assistant') - expect(model.primary[1].to).toBe('/assistant') + expect(model.primary[1].to).toBe('/assistant?view=sessions') expect(model.primary[1].variant).toBe('secondary') }) @@ -120,7 +120,7 @@ describe('buildNavModel', () => { expect(model.primary[0].onSelect).toBe('new-session') expect(model.primary[0].variant).toBe('primary') expect(model.primary[1].key).toBe('assistant') - expect(model.primary[1].to).toBe('/assistant') + expect(model.primary[1].to).toBe('/assistant?view=sessions') expect(model.primary[1].variant).toBe('secondary') }) @@ -131,7 +131,7 @@ describe('buildNavModel', () => { expect(model.primary[0].onSelect).toBe('new-session') expect(model.primary[0].variant).toBe('primary') expect(model.primary[1].key).toBe('assistant') - expect(model.primary[1].to).toBe('/assistant') + expect(model.primary[1].to).toBe('/assistant?view=sessions') expect(model.primary[1].variant).toBe('secondary') }) @@ -141,21 +141,21 @@ describe('buildNavModel', () => { expect(model1.primary[0].key).toBe('new-schedule') expect(model1.primary[0].onSelect).toBe('new-schedule') expect(model1.primary[1].key).toBe('assistant') - expect(model1.primary[1].to).toBe('/assistant') + expect(model1.primary[1].to).toBe('/assistant?view=sessions') const model2 = buildNavModel('/repos/5/schedules') expect(model2.primary).toHaveLength(2) expect(model2.primary[0].key).toBe('new-schedule') expect(model2.primary[0].onSelect).toBe('new-schedule') expect(model2.primary[1].key).toBe('assistant') - expect(model2.primary[1].to).toBe('/assistant') + expect(model2.primary[1].to).toBe('/assistant?view=sessions') }) it('returns assistant primary for unknown routes', () => { const model = buildNavModel('/unknown/path') expect(model.primary).toHaveLength(1) expect(model.primary[0].key).toBe('assistant') - expect(model.primary[0].to).toBe('/assistant') + expect(model.primary[0].to).toBe('/assistant?view=sessions') }) it('preserves backwards compatibility with buildMoreItems', () => { diff --git a/frontend/src/components/navigation/moreDrawerItems.ts b/frontend/src/components/navigation/moreDrawerItems.ts index 23d451cf..8d7c0b3e 100644 --- a/frontend/src/components/navigation/moreDrawerItems.ts +++ b/frontend/src/components/navigation/moreDrawerItems.ts @@ -1,6 +1,6 @@ import type { LucideIcon } from 'lucide-react' import { Plug, Sparkles, ShieldOff, CalendarClock, GitCommitHorizontal, Code2, Settings, LogOut, Plus, Bot, Folder, Clock, SquarePlus } from 'lucide-react' -import { getAssistantPath, isAssistantPath } from '@/lib/navigation' +import { getAssistantSessionListPath, isAssistantPath } from '@/lib/navigation' export interface MoreDrawerItem { key: string @@ -30,7 +30,7 @@ function getAssistantNavItem(_pathname: string, variant: NavPrimaryCta['variant' key: 'assistant', label: 'Assistant', icon: Bot, - to: getAssistantPath(), + to: getAssistantSessionListPath(), variant, } } diff --git a/frontend/src/hooks/__tests__/useLoadSkill.test.tsx b/frontend/src/hooks/__tests__/useLoadSkill.test.tsx index 51f53c1f..2b11f1bd 100644 --- a/frontend/src/hooks/__tests__/useLoadSkill.test.tsx +++ b/frontend/src/hooks/__tests__/useLoadSkill.test.tsx @@ -5,6 +5,7 @@ import { useLoadSkill } from '../useOpenCode' import type { MessageWithParts } from '../../api/types' import { showToast } from '../../lib/toast' +import { useSessionStatus } from '../../stores/sessionStatusStore' const mocks = vi.hoisted(() => ({ sendCommand: vi.fn(), @@ -38,6 +39,7 @@ const createWrapper = () => { describe('useLoadSkill', () => { beforeEach(() => { vi.clearAllMocks() + useSessionStatus.getState().replaceStatuses({}) }) it('calls sendCommand with correct parameters when mutate is called', async () => { @@ -161,13 +163,29 @@ describe('useLoadSkill', () => { }) }) - it('sets session status to busy on mutate and idle on error', async () => { + it('sets session status to busy on mutate', async () => { + mocks.sendCommand.mockReturnValue(new Promise(() => {})) + + const { result } = renderHook( + () => useLoadSkill('http://localhost:5551', 'test-session-id', '/test/dir'), + { wrapper: createWrapper() }, + ) + + result.current.mutate({ skillName: 'my-skill' }) + + await waitFor(() => { + expect(useSessionStatus.getState().getStatus('test-session-id').type).toBe('busy') + }) + }) + + it('rolls back to idle on mutation error', async () => { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false }, mutations: { retry: false }, }, }) + const testError = new Error('Command failed') mocks.sendCommand.mockRejectedValue(testError) @@ -182,6 +200,33 @@ describe('useLoadSkill', () => { expect(result.current.isError).toBe(true) }) + expect(useSessionStatus.getState().getStatus('test-session-id').type).toBe('idle') + }) + + it('does not set session status when sessionID is undefined', async () => { + const queryClient = new QueryClient({ + defaultOptions: { + queries: { retry: false }, + mutations: { retry: false }, + }, + }) + + mocks.sendCommand.mockReturnValue(new Promise(() => {})) + + const { result } = renderHook( + () => useLoadSkill('http://localhost:5551', undefined, '/test/dir'), + { wrapper: ({ children }) => {children} }, + ) + + result.current.mutate({ skillName: 'my-skill' }) + + await waitFor(() => { + expect(result.current.isError).toBe(true) + }) + + expect(result.current.error).toBeInstanceOf(Error) + expect((result.current.error as Error).message).toBe('No active session') + expect(useSessionStatus.getState().getStatus('test-session-id').type).toBe('idle') }) it('removes optimistic message from cache on error', async () => { diff --git a/frontend/src/hooks/useOpenCode.ts b/frontend/src/hooks/useOpenCode.ts index e0122c32..382a4d31 100644 --- a/frontend/src/hooks/useOpenCode.ts +++ b/frontend/src/hooks/useOpenCode.ts @@ -12,6 +12,8 @@ import type { paths, components } from "../api/opencode-types"; import { parseNetworkError } from "../lib/opencode-errors"; import { showToast } from "../lib/toast"; import { useSendErrorStore } from "../stores/sendErrorStore"; +import { beginOptimisticBusy, rollbackOptimisticBusy } from "../stores/sessionStatusStore"; +import type { SessionStatusType } from "../stores/sessionStatusStore"; import { invalidateSessionListCaches, messagesQueryKey } from "../lib/queryInvalidation"; type AssistantMessage = components["schemas"]["AssistantMessage"]; @@ -22,6 +24,8 @@ type SendPromptRequest = NonNullable< type SendCommandResponse = paths["/session/{sessionID}/command"]["post"]["responses"]["200"]["content"]["application/json"]; +type OptimisticBusyContext = { previousBusyStatus: SessionStatusType }; + const parseModelString = (model: string) => { const [providerID, ...rest] = model.split("/"); const modelID = rest.join("/"); @@ -477,8 +481,13 @@ export const useSendPrompt = (opcodeUrl: string | null | undefined, directory?: return { optimisticUserID, response, queued: false }; }, - onError: (error, variables) => { + onMutate: ({ sessionID }): OptimisticBusyContext => ({ + previousBusyStatus: beginOptimisticBusy(sessionID), + }), + onError: (error, variables, context: OptimisticBusyContext | undefined) => { const { sessionID } = variables; + rollbackOptimisticBusy(sessionID, context?.previousBusyStatus ?? { type: 'idle' }); + const queryKey = messagesQueryKey(opcodeUrl, sessionID, directory); queryClient.setQueryData( @@ -726,8 +735,12 @@ export const useSendShell = (opcodeUrl: string | null | undefined, directory?: s return { optimisticUserID, response }; }, - onError: (_, variables) => { + onMutate: ({ sessionID }): OptimisticBusyContext => ({ + previousBusyStatus: beginOptimisticBusy(sessionID), + }), + onError: (_error, variables, context: OptimisticBusyContext | undefined) => { const { sessionID } = variables; + rollbackOptimisticBusy(sessionID, context?.previousBusyStatus ?? { type: 'idle' }); queryClient.setQueryData( messagesQueryKey(opcodeUrl, sessionID, directory), (old) => { @@ -788,7 +801,8 @@ export const useLoadSkill = ( return useMutation< { optimisticUserID: string; response: SendCommandResponse }, Error, - { skillName: string } + { skillName: string }, + OptimisticBusyContext | undefined >({ mutationFn: async ({ skillName }: { skillName: string }) => { if (!client) throw new Error("No OpenCode client available"); @@ -817,7 +831,14 @@ export const useLoadSkill = ( const response = await client.sendCommand(sessionID, { command: skillName, arguments: "" }); return { optimisticUserID, response }; }, - onError: (error) => { + onMutate: () => { + if (!sessionID) return undefined; + return { previousBusyStatus: beginOptimisticBusy(sessionID) }; + }, + onError: (error, _variables, context: OptimisticBusyContext | undefined) => { + if (sessionID && context?.previousBusyStatus) { + rollbackOptimisticBusy(sessionID, context.previousBusyStatus); + } if (sessionID) { const queryKey = messagesQueryKey(opcodeUrl, sessionID, directory); queryClient.setQueryData( diff --git a/frontend/src/hooks/useSendPrompt.test.ts b/frontend/src/hooks/useSendPrompt.test.ts index 4c36a341..8423a318 100644 --- a/frontend/src/hooks/useSendPrompt.test.ts +++ b/frontend/src/hooks/useSendPrompt.test.ts @@ -1,9 +1,10 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' -import { renderHook } from '@testing-library/react' +import { renderHook, waitFor } from '@testing-library/react' import { QueryClient, QueryClientProvider } from '@tanstack/react-query' import { createElement } from 'react' import { useSendPrompt } from './useOpenCode' import { FetchError } from '../api/fetchWrapper' +import { useSessionStatus } from '../stores/sessionStatusStore' const mockSendPrompt = vi.fn() const mockSendPromptAsync = vi.fn() @@ -19,10 +20,6 @@ vi.mock('../api/opencode', async () => { } }) -vi.mock('@/stores/sessionStatusStore', () => ({ - useSessionStatus: vi.fn(() => vi.fn()), -})) - vi.mock('../lib/toast', () => ({ showToast: { error: vi.fn() }, })) @@ -62,6 +59,7 @@ describe('useSendPrompt', () => { beforeEach(() => { vi.clearAllMocks() + useSessionStatus.getState().replaceStatuses({}) queryClient = createTestQueryClient() mockSendPrompt.mockResolvedValue({ info: { id: 'test-response' }, @@ -194,4 +192,25 @@ describe('useSendPrompt', () => { expect(mockClearError).toHaveBeenCalledWith('session-2') }) + + it('sets session status to busy immediately on send', async () => { + mockSendPrompt.mockReturnValue(new Promise(() => {})) + + const { result } = renderHookWithProviders() + result.current.mutate({ sessionID: 'busy-session', prompt: 'Hi' }) + + await waitFor(() => { + expect(useSessionStatus.getState().getStatus('busy-session').type).toBe('busy') + }) + }) + + it('rolls back to idle when send fails', async () => { + mockSendPrompt.mockRejectedValue(new Error('boom')) + + const { result } = renderHookWithProviders() + result.current.mutate({ sessionID: 'busy-session', prompt: 'Hi' }) + + await waitFor(() => expect(result.current.isError).toBe(true)) + expect(useSessionStatus.getState().getStatus('busy-session').type).toBe('idle') + }) }) diff --git a/frontend/src/hooks/useSendShell.test.ts b/frontend/src/hooks/useSendShell.test.ts new file mode 100644 index 00000000..e62ec90a --- /dev/null +++ b/frontend/src/hooks/useSendShell.test.ts @@ -0,0 +1,71 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { renderHook, waitFor } from '@testing-library/react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { createElement } from 'react' +import { useSendShell } from './useOpenCode' +import { useSessionStatus } from '../stores/sessionStatusStore' + +const mockSendShell = vi.fn() + +vi.mock('../api/opencode', async () => { + const actual = await vi.importActual('../api/opencode') + return { + ...actual, + OpenCodeClient: vi.fn().mockImplementation(() => ({ + sendShell: mockSendShell, + })), + } +}) + +vi.mock('../lib/toast', () => ({ + showToast: { error: vi.fn() }, +})) + +const createTestQueryClient = () => + new QueryClient({ + defaultOptions: { + queries: { + retry: false, + }, + }, + }) + +describe('useSendShell', () => { + let queryClient: QueryClient + + beforeEach(() => { + vi.clearAllMocks() + useSessionStatus.getState().replaceStatuses({}) + queryClient = createTestQueryClient() + }) + + const renderHookWithProviders = () => + renderHook( + () => useSendShell('http://localhost:5551', '/test'), + { + wrapper: ({ children }) => + createElement(QueryClientProvider, { client: queryClient }, children), + } + ) + + it('sets session status to busy on shell send', async () => { + mockSendShell.mockReturnValue(new Promise(() => {})) + + const { result } = renderHookWithProviders() + result.current.mutate({ sessionID: 'shell-session', command: 'ls', agent: 'general' }) + + await waitFor(() => { + expect(useSessionStatus.getState().getStatus('shell-session').type).toBe('busy') + }) + }) + + it('rolls back to idle when shell send fails', async () => { + mockSendShell.mockRejectedValue(new Error('boom')) + + const { result } = renderHookWithProviders() + result.current.mutate({ sessionID: 'shell-session', command: 'ls', agent: 'general' }) + + await waitFor(() => expect(result.current.isError).toBe(true)) + expect(useSessionStatus.getState().getStatus('shell-session').type).toBe('idle') + }) +}) diff --git a/frontend/src/pages/RepoDetail.tsx b/frontend/src/pages/RepoDetail.tsx index a9de77b0..dd53cc22 100644 --- a/frontend/src/pages/RepoDetail.tsx +++ b/frontend/src/pages/RepoDetail.tsx @@ -108,7 +108,11 @@ export function RepoDetail() { const sessionUrl = useCallback( (sessionId: string) => { const base = `/repos/${repoId}/sessions/${sessionId}`; - return activeTab === 'workspaces' ? `${base}?repoTab=workspaces` : base; + const params = new URLSearchParams(); + if (repoId === 0) params.set('assistant', '1'); + if (activeTab === 'workspaces') params.set('repoTab', 'workspaces'); + const search = params.toString(); + return search ? `${base}?${search}` : base; }, [repoId, activeTab], ); diff --git a/frontend/src/stores/sessionStatusStore.test.ts b/frontend/src/stores/sessionStatusStore.test.ts new file mode 100644 index 00000000..f1b4042e --- /dev/null +++ b/frontend/src/stores/sessionStatusStore.test.ts @@ -0,0 +1,34 @@ +import { describe, it, expect, beforeEach } from 'vitest' +import { useSessionStatus, beginOptimisticBusy, rollbackOptimisticBusy } from './sessionStatusStore' + +describe('optimistic busy helpers', () => { + beforeEach(() => { + useSessionStatus.getState().replaceStatuses({}) + }) + + it('beginOptimisticBusy on idle session sets busy and returns idle', () => { + const previous = beginOptimisticBusy('session-1') + expect(previous).toEqual({ type: 'idle' }) + expect(useSessionStatus.getState().getStatus('session-1')).toEqual({ type: 'busy' }) + }) + + it('rollbackOptimisticBusy returns session to idle', () => { + beginOptimisticBusy('session-1') + rollbackOptimisticBusy('session-1', { type: 'idle' }) + expect(useSessionStatus.getState().getStatus('session-1')).toEqual({ type: 'idle' }) + }) + + it('beginOptimisticBusy on already-busy session returns busy and keeps busy', () => { + useSessionStatus.getState().setStatus('session-1', { type: 'busy' }) + const previous = beginOptimisticBusy('session-1') + expect(previous).toEqual({ type: 'busy' }) + expect(useSessionStatus.getState().getStatus('session-1')).toEqual({ type: 'busy' }) + rollbackOptimisticBusy('session-1', { type: 'busy' }) + expect(useSessionStatus.getState().getStatus('session-1')).toEqual({ type: 'busy' }) + }) + + it('busy state is scoped per sessionID', () => { + beginOptimisticBusy('a') + expect(useSessionStatus.getState().getStatus('b')).toEqual({ type: 'idle' }) + }) +}) diff --git a/frontend/src/stores/sessionStatusStore.ts b/frontend/src/stores/sessionStatusStore.ts index 823ba340..a7f4ef14 100644 --- a/frontend/src/stores/sessionStatusStore.ts +++ b/frontend/src/stores/sessionStatusStore.ts @@ -104,3 +104,17 @@ export const useSessionStatusForSession = (sessionID: string | undefined): Sessi sessionID ? (state.statuses.get(sessionID) ?? DEFAULT_STATUS) : DEFAULT_STATUS ) } + +export function beginOptimisticBusy(sessionID: string): SessionStatusType { + const { getStatus, setStatus } = useSessionStatus.getState() + const previous = getStatus(sessionID) + setStatus(sessionID, { type: 'busy' }) + return previous +} + +export function rollbackOptimisticBusy( + sessionID: string, + previous: SessionStatusType, +): void { + useSessionStatus.getState().setStatus(sessionID, previous) +}