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 638cc67f8..fb266b758 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 c447cab31..187609f65 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 9dc50a366..466b770b8 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 351ffa4b5..b2c8ebe56 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 eeb572abc..fc9bc42e8 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 000000000..09233b6cc --- /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 f03d6455c..c6169e1ea 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 d5f098ec8..bb5bab03a 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(),