From 217d1ee1a8fdbfc642f0f1d2efd747768e461f9f Mon Sep 17 00:00:00 2001 From: Alexander Dreweke Date: Mon, 27 Jul 2026 07:28:00 +0200 Subject: [PATCH 1/2] Fix question prompt not rendering for active session currentQuestion was the first pending question globally across all sessions. When another session had the first question in the flat list, the QuestionPrompt would not render for the session being viewed even though it had its own pending question. Add getForSession() to the questions context and use it in SessionDetail to look up questions scoped to the current session. --- frontend/src/contexts/EventContext.tsx | 7 +++++++ frontend/src/pages/SessionDetail.tsx | 5 +++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/frontend/src/contexts/EventContext.tsx b/frontend/src/contexts/EventContext.tsx index 638cc67f..fb266b75 100644 --- a/frontend/src/contexts/EventContext.tsx +++ b/frontend/src/contexts/EventContext.tsx @@ -158,6 +158,7 @@ interface EventContextValue { reject: (requestID: string) => Promise dismiss: (requestID: string, sessionID?: string) => void getForCallID: (callID: string, sessionID: string) => QuestionRequest | null + getForSession: (sessionID: string) => QuestionRequest | null hasForSession: (sessionID: string) => boolean navigateToCurrent: () => void syncForSession: (directory: string, sessionID: string) => Promise @@ -404,6 +405,10 @@ export function EventProvider({ children }: { children: React.ReactNode }) { return (permissionsBySession[sessionID]?.length ?? 0) > 0 }, [permissionsBySession]) + const getQuestionForSession = useCallback((sessionID: string): QuestionRequest | null => { + return questionsBySession[sessionID]?.[0] ?? null + }, [questionsBySession]) + const hasQuestionsForSession = useCallback((sessionID: string): boolean => { return (questionsBySession[sessionID]?.length ?? 0) > 0 }, [questionsBySession]) @@ -597,6 +602,7 @@ export function EventProvider({ children }: { children: React.ReactNode }) { reject: rejectQuestion, dismiss: removeQuestion, getForCallID: getQuestionForCallID, + getForSession: getQuestionForSession, hasForSession: hasQuestionsForSession, navigateToCurrent: navigateToCurrentQuestion, syncForSession: syncQuestionsForSession, @@ -622,6 +628,7 @@ export function EventProvider({ children }: { children: React.ReactNode }) { rejectQuestion, removeQuestion, getQuestionForCallID, + getQuestionForSession, hasQuestionsForSession, navigateToCurrentQuestion, syncQuestionsForSession, diff --git a/frontend/src/pages/SessionDetail.tsx b/frontend/src/pages/SessionDetail.tsx index c447cab3..187609f6 100644 --- a/frontend/src/pages/SessionDetail.tsx +++ b/frontend/src/pages/SessionDetail.tsx @@ -173,7 +173,8 @@ export function SessionDetail() { const { isEnabled: ttsEnabled } = useTTS(); const sessionStatus = useSessionStatusForSession(sessionId); const { syncForSession: syncPermissionsForSession } = usePermissions(); - const { current: currentQuestion, reply: replyToQuestion, reject: rejectQuestion, syncForSession: syncQuestionsForSession } = useQuestions(); + const { getForSession: getQuestionForSession, reply: replyToQuestion, reject: rejectQuestion, syncForSession: syncQuestionsForSession } = useQuestions(); + const currentQuestion = sessionId ? getQuestionForSession(sessionId) : null; const lastAssistantMessage = messages?.filter(m => m.info.role === 'assistant').at(-1); const lastAssistantText = getAssistantText(lastAssistantMessage); @@ -568,7 +569,7 @@ export function SessionDetail() { onDismiss={() => rejectQuestion(minimizedQuestion.id)} /> )} - {!minimizedQuestion && currentQuestion && currentQuestion.sessionID === sessionId && ( + {!minimizedQuestion && currentQuestion && ( Date: Tue, 28 Jul 2026 19:24:36 -0400 Subject: [PATCH 2/2] test(frontend): cover session-scoped question prompt resolution The SessionDetail useQuestions mocks did not expose getForSession, so every page test threw on the new session-scoped lookup. Add it to each mock and lock in the behaviour with regression coverage: the context helper resolves questions per session while another session owns the global current question, and SessionDetail renders the prompt for the session being viewed. --- frontend/src/contexts/EventContext.test.tsx | 22 +- .../SessionDetail.assistant-loading.test.tsx | 1 + .../__tests__/SessionDetail.polling.test.tsx | 1 + .../SessionDetail.question-prompt.test.tsx | 278 ++++++++++++++++++ .../SessionDetail.scroll-floating.test.tsx | 1 + .../SessionDetail.todo-header.test.tsx | 1 + 6 files changed, 303 insertions(+), 1 deletion(-) create mode 100644 frontend/src/pages/__tests__/SessionDetail.question-prompt.test.tsx diff --git a/frontend/src/contexts/EventContext.test.tsx b/frontend/src/contexts/EventContext.test.tsx index 9dc50a36..466b770b 100644 --- a/frontend/src/contexts/EventContext.test.tsx +++ b/frontend/src/contexts/EventContext.test.tsx @@ -94,7 +94,7 @@ const pendingPermission: PermissionRequest = { } function Harness() { - const { current, pendingCount, syncForSession, navigateToCurrent, reject, reply } = useQuestions() + const { current, pendingCount, syncForSession, navigateToCurrent, reject, reply, getForSession } = useQuestions() const permissions = usePermissions() const permissionForCall = permissions.getForCallID('call-1', 'session-1') const location = useLocation() @@ -103,6 +103,9 @@ function Harness() {
{pendingCount}
{current?.id ?? 'none'}
+
{getForSession('session-1')?.id ?? 'none'}
+
{getForSession('session-2')?.id ?? 'none'}
+
{getForSession('session-unknown')?.id ?? 'none'}
{permissions.pendingCount}
{permissions.current?.id ?? 'none'}
{permissionForCall?.id ?? 'none'}
@@ -239,6 +242,23 @@ describe('EventProvider questions', () => { }) }) + it('resolves pending questions per session independently of the global current', async () => { + mocks.listPendingQuestions.mockResolvedValue([pendingQuestion, secondPendingQuestion]) + + render(, { wrapper: createWrapper() }) + + await userEvent.click(screen.getByRole('button', { name: 'Sync' })) + + await waitFor(() => { + expect(screen.getByTestId('count')).toHaveTextContent('2') + }) + + expect(screen.getByTestId('current')).toHaveTextContent('question-1') + expect(screen.getByTestId('for-session-1')).toHaveTextContent('question-1') + expect(screen.getByTestId('for-session-2')).toHaveTextContent('question-2') + expect(screen.getByTestId('for-session-unknown')).toHaveTextContent('none') + }) + it('reconciles stale pending questions after reconnect', async () => { mocks.listRepos.mockResolvedValue([{ id: 123, fullPath: '/repo' }]) mocks.listPendingQuestions diff --git a/frontend/src/pages/__tests__/SessionDetail.assistant-loading.test.tsx b/frontend/src/pages/__tests__/SessionDetail.assistant-loading.test.tsx index 351ffa4b..b2c8ebe5 100644 --- a/frontend/src/pages/__tests__/SessionDetail.assistant-loading.test.tsx +++ b/frontend/src/pages/__tests__/SessionDetail.assistant-loading.test.tsx @@ -185,6 +185,7 @@ describe('SessionDetail assistant loading at repoId=0', () => { }) mocks.useQuestions.mockReturnValue({ current: null, + getForSession: vi.fn(() => null), pendingCount: 0, hasQuestionsForSession: vi.fn(() => false), reply: vi.fn(), diff --git a/frontend/src/pages/__tests__/SessionDetail.polling.test.tsx b/frontend/src/pages/__tests__/SessionDetail.polling.test.tsx index eeb572ab..fc9bc42e 100644 --- a/frontend/src/pages/__tests__/SessionDetail.polling.test.tsx +++ b/frontend/src/pages/__tests__/SessionDetail.polling.test.tsx @@ -164,6 +164,7 @@ describe('SessionDetail pending-actions polling gating', () => { }) mocks.useQuestions.mockReturnValue({ current: null, + getForSession: vi.fn(() => null), pendingCount: 0, hasQuestionsForSession: vi.fn(() => false), reply: vi.fn(), diff --git a/frontend/src/pages/__tests__/SessionDetail.question-prompt.test.tsx b/frontend/src/pages/__tests__/SessionDetail.question-prompt.test.tsx new file mode 100644 index 00000000..09233b6c --- /dev/null +++ b/frontend/src/pages/__tests__/SessionDetail.question-prompt.test.tsx @@ -0,0 +1,278 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { render, screen, waitFor } from '@testing-library/react' +import { QueryClient, QueryClientProvider } from '@tanstack/react-query' +import { MemoryRouter, Route, Routes } from 'react-router-dom' +import type { QuestionRequest } from '@/api/types' +import { SessionDetail } from '../SessionDetail' + +const mocks = vi.hoisted(() => ({ + useSession: vi.fn(), + useMessages: vi.fn(), + useSSE: vi.fn(), + useRepoActivity: vi.fn(), + usePermissions: vi.fn(), + useQuestions: vi.fn(), + useSSEHealth: vi.fn(), + useConfig: vi.fn(), + useOpenCodeClient: vi.fn(), + useMobile: vi.fn(), + useAutoScroll: vi.fn(), + useDialogParam: vi.fn(), + useSidebarAction: vi.fn(), + useSessionStatusForSession: vi.fn(), +})) + +vi.mock('@/config', () => ({ + OPENCODE_API_ENDPOINT: 'http://localhost:5551/api/opencode', + API_BASE_URL: 'http://localhost:5551', + SERVER_PORT: 5003, + OPENCODE_PORT: 5551, + FILE_LIMITS: {}, + DEFAULTS: {}, + ALLOWED_MIME_TYPES: [], + GIT_PROVIDERS: [], +})) + +vi.mock('@/hooks/useOpenCode', () => ({ + useSession: mocks.useSession, + useAbortSession: vi.fn(() => ({ mutate: vi.fn() })), + useUpdateSession: vi.fn(() => ({ mutate: vi.fn() })), + useCreateSession: vi.fn(() => ({ mutateAsync: vi.fn() })), + useMessages: mocks.useMessages, + useConfig: mocks.useConfig, + useSendPrompt: vi.fn(() => ({ mutate: vi.fn() })), + useSendShell: vi.fn(() => ({ mutate: vi.fn() })), + useAgents: vi.fn(() => ({ data: [] })), + useOpenCodeClient: mocks.useOpenCodeClient, +})) + +vi.mock('@/hooks/useModelSelection', () => ({ + useModelSelection: vi.fn(() => ({ model: null, modelString: null })), +})) + +vi.mock('@/hooks/useTTS', () => ({ + useTTS: vi.fn(() => ({ isEnabled: false })), +})) + +vi.mock('@/hooks/useSettings', () => ({ + useSettings: vi.fn(() => ({ + preferences: { expandToolCalls: false }, + updateSettings: vi.fn(), + })), +})) + +vi.mock('@/hooks/useSettingsDialog', () => ({ + useSettingsDialog: vi.fn(() => ({ open: vi.fn() })), +})) + +vi.mock('@/hooks/useMobile', () => ({ + useMobile: mocks.useMobile, + useSwipeBack: vi.fn(() => ({ ref: vi.fn() })), +})) + +vi.mock('@/hooks/useVisualViewport', () => ({ + useVisualViewport: vi.fn(() => ({ keyboardHeight: 0 })), +})) + +vi.mock('@/hooks/useKeyboardShortcuts', () => ({ + useKeyboardShortcuts: vi.fn(() => ({ leaderActive: false })), +})) + +vi.mock('@/hooks/useAutoScroll', () => ({ + useAutoScroll: mocks.useAutoScroll, +})) + +vi.mock('@/hooks/useDialogParam', () => ({ + useDialogParam: vi.fn(() => [false, vi.fn()]), +})) + +vi.mock('@/hooks/useSidebarAction', () => ({ + useSidebarAction: vi.fn(() => {}), +})) + +vi.mock('@/hooks/useAutoPlayLastResponse', () => ({ + getAssistantText: vi.fn(() => ''), + getLatestPlayableAssistantMessage: vi.fn(() => null), + useAutoPlayLastResponse: vi.fn(() => {}), +})) + +vi.mock('@/stores/uiStateStore', () => ({ + useUIState: vi.fn((selector?: (state: Record) => unknown) => + typeof selector === 'function' + ? selector({ isEditingMessage: false, setActivePromptFileBasePath: vi.fn() }) + : false + ), +})) + +vi.mock('@/stores/sessionStatusStore', () => ({ + useSessionStatus: vi.fn(() => ({ setStatus: vi.fn() })), + useSessionStatusForSession: mocks.useSessionStatusForSession, +})) + +vi.mock('@/hooks/useSSE', () => ({ + useSSE: mocks.useSSE, +})) + +vi.mock('@/hooks/useRepoActivity', () => ({ + useRepoActivity: mocks.useRepoActivity, +})) + +vi.mock('@/contexts/EventContext', async (importOriginal) => { + const actual = await importOriginal() + return { + ...(actual as object), + usePermissions: mocks.usePermissions, + useQuestions: mocks.useQuestions, + useSSEHealth: mocks.useSSEHealth, + } +}) + +vi.mock('@/api/repos', () => ({ + getRepo: vi.fn(() => Promise.resolve({ + id: 1, + repoUrl: 'https://github.com/test/repo', + localPath: '/test/repo', + sourcePath: null, + fullPath: '/test/repo', + branch: 'main', + currentBranch: 'main', + fullSlug: 'test/repo', + repoType: 'github' as const, + })), + initializeAssistantMode: vi.fn(() => Promise.resolve({ directory: '/test/repo' })), +})) + +vi.mock('@/components/model/ModelSelectDialog', () => ({ + ModelSelectDialog: vi.fn(() => null), +})) + +vi.mock('@/components/session/SessionList', () => ({ + SessionList: vi.fn(() => null), +})) + +vi.mock('@/components/file-browser/FileBrowserSheet', () => ({ + FileBrowserSheet: vi.fn(() => null), +})) + +vi.mock('@/components/repo/RepoMcpDialog', () => ({ + RepoMcpDialog: vi.fn(() => null), +})) + +vi.mock('@/components/repo/ResetPermissionsDialog', () => ({ + ResetPermissionsDialog: vi.fn(() => null), +})) + +vi.mock('@/components/repo/RepoLspDialog', () => ({ + RepoLspDialog: vi.fn(() => null), +})) + +vi.mock('@/components/repo/RepoSkillsDialog', () => ({ + RepoSkillsDialog: vi.fn(() => null), +})) + +vi.mock('@/components/source-control', () => ({ + SourceControlPanel: vi.fn(() => null), +})) + +vi.mock('@/components/session/QuestionPrompt', () => ({ + QuestionPrompt: ({ question }: { question: QuestionRequest }) => ( +
{question.id}
+ ), +})) + +vi.mock('@/components/session/MinimizedQuestionIndicator', () => ({ + MinimizedQuestionIndicator: vi.fn(() => null), +})) + +vi.mock('@/components/notifications/PendingActionsGroup', () => ({ + PendingActionsGroup: vi.fn(() => null), +})) + +vi.mock('@/components/message/PromptInput', () => ({ + PromptInput: vi.fn(() =>
MockedPromptInput
), +})) + +const VIEWED_SESSION_ID = 'viewed-session' + +function createQuestion(id: string, sessionID: string): QuestionRequest { + return { + id, + sessionID, + questions: [ + { + question: 'Continue?', + header: 'Confirm', + options: [{ label: 'Yes', description: 'Continue' }], + multiple: false, + }, + ], + } +} + +const viewedSessionQuestion = createQuestion('question-viewed', VIEWED_SESSION_ID) +const otherSessionQuestion = createQuestion('question-other', 'other-session') + +describe('SessionDetail question prompt session scoping', () => { + beforeEach(() => { + vi.clearAllMocks() + + mocks.useSession.mockReturnValue({ data: undefined, isLoading: false }) + mocks.useMessages.mockReturnValue({ data: [], isLoading: false }) + mocks.useSSE.mockReturnValue({ isConnected: true, isReconnecting: false }) + mocks.useRepoActivity.mockReturnValue(undefined) + mocks.usePermissions.mockReturnValue({ + pendingCount: 0, + syncForSession: vi.fn(), + }) + mocks.useSSEHealth.mockReturnValue({ isHealthy: true }) + mocks.useConfig.mockReturnValue({ data: undefined, isLoading: false }) + mocks.useOpenCodeClient.mockReturnValue({}) + mocks.useMobile.mockReturnValue(false) + mocks.useAutoScroll.mockReturnValue({ scrollToBottom: vi.fn() }) + mocks.useDialogParam.mockReturnValue([false, vi.fn()]) + mocks.useSidebarAction.mockReturnValue(undefined) + mocks.useSessionStatusForSession.mockReturnValue({ type: 'idle' }) + }) + + const renderWithQuestions = (questionsBySession: Record, current: QuestionRequest | null) => { + mocks.useQuestions.mockReturnValue({ + current, + getForSession: vi.fn((sessionID: string) => questionsBySession[sessionID] ?? null), + pendingCount: Object.keys(questionsBySession).length, + reply: vi.fn(), + reject: vi.fn(), + syncForSession: vi.fn(), + }) + + return render( + + + + } /> + + + + ) + } + + it('renders the viewed session question when another session owns the globally current question', async () => { + renderWithQuestions( + { + [VIEWED_SESSION_ID]: viewedSessionQuestion, + 'other-session': otherSessionQuestion, + }, + otherSessionQuestion + ) + + await waitFor(() => { + expect(screen.getByTestId('question-prompt')).toHaveTextContent('question-viewed') + }) + }) + + it('renders no question prompt when only another session has a pending question', async () => { + renderWithQuestions({ 'other-session': otherSessionQuestion }, otherSessionQuestion) + + await waitFor(() => expect(screen.getByText('MockedPromptInput')).toBeInTheDocument()) + expect(screen.queryByTestId('question-prompt')).not.toBeInTheDocument() + }) +}) diff --git a/frontend/src/pages/__tests__/SessionDetail.scroll-floating.test.tsx b/frontend/src/pages/__tests__/SessionDetail.scroll-floating.test.tsx index f03d6455..c6169e1e 100644 --- a/frontend/src/pages/__tests__/SessionDetail.scroll-floating.test.tsx +++ b/frontend/src/pages/__tests__/SessionDetail.scroll-floating.test.tsx @@ -214,6 +214,7 @@ describe('SessionDetail scroll floating button', () => { }) mocks.useQuestions.mockReturnValue({ current: null, + getForSession: vi.fn(() => null), pendingCount: 0, hasQuestionsForSession: vi.fn(() => false), reply: vi.fn(), diff --git a/frontend/src/pages/__tests__/SessionDetail.todo-header.test.tsx b/frontend/src/pages/__tests__/SessionDetail.todo-header.test.tsx index d5f098ec..bb5bab03 100644 --- a/frontend/src/pages/__tests__/SessionDetail.todo-header.test.tsx +++ b/frontend/src/pages/__tests__/SessionDetail.todo-header.test.tsx @@ -197,6 +197,7 @@ describe('SessionDetail todo-header integration', () => { }) mocks.useQuestions.mockReturnValue({ current: null, + getForSession: vi.fn(() => null), pendingCount: 0, hasQuestionsForSession: vi.fn(() => false), reply: vi.fn(),