From 5584537da58df6fab3a1af03602c8697d5f43505 Mon Sep 17 00:00:00 2001 From: jwcrystal <121911854+jwcrystal@users.noreply.github.com> Date: Wed, 15 Apr 2026 01:02:07 +0800 Subject: [PATCH] fix: question tool content disappears after refresh (#879) (#909) * fix: show question content in ToolPart instead of 'Awaiting response...' after refresh Previously, when the question tool was pending/running or completed without parseable output, the ToolPart fell through to a generic 'Awaiting response...' message. After a page refresh or app restart, this made questions appear empty even though the tool state still contained the question input data. Now the ToolPart reads question text, headers, and options from the tool state's input field, ensuring question content persists across refreshes regardless of QuestionCard store availability. Fixes #879 * fix: restore QuestionCard after refresh and pause working status during active questions Two fixes for question tool UX: 1. ChatContainer: sessionIsWorking now returns false when there are active questions (same as it already did for permissions). This prevents the status row from showing 'Asking question...' and instead shows the QuestionCard. 2. sync-context: resyncDirectoryAfterReconnect now re-fetches pending questions via listPendingQuestions(). Previously only sessions and messages were re-fetched on SSE reconnect, so questions asked during disconnection were lost, causing QuestionCard to disappear after page refresh. Refs #879 * fix: hide assistant working status while questions are pending The assistant status hook only special-cased pending permissions, so question tools still surfaced 'Asking question...' after refresh even when the UI was already waiting on a QuestionCard response. Treat pending questions like other blocking requests by clearing the working indicator until the user answers. Refs #879 * fix: merge question/permission stores instead of full replace on bootstrap and reconnect The root cause of QuestionCard disappearing after refresh was a race condition between SSE events and HTTP bootstrap. Bootstrap and reconnect both did full replacement of state.question, wiping SSE-delivered data that arrived between the HTTP call initiation and response arrival. Changes: - bootstrap.ts: question and permission stores now use merge semantics. Only sessions present in the API response are overwritten. Sessions absent from the response are left untouched (they may hold SSE data). - sync-context.tsx: reconnect question resync uses the same merge pattern. No longer clears question entries for sessions not in the API response. - bootstrap.ts: sdk.question.list() now passes directory parameter to scope the query correctly. This ensures SSE-delivered question data survives the bootstrap window, while still allowing the API response to be authoritative for sessions it covers. Refs #879 * fix: prune stale pending requests after reconnect --------- Co-authored-by: Bohdan Triapitsyn --- .../ui/src/components/chat/ChatContainer.tsx | 4 +- .../chat/message/parts/ToolPart.tsx | 31 ++++++- packages/ui/src/hooks/useAssistantStatus.ts | 22 ++++- packages/ui/src/sync/bootstrap.ts | 80 ++++++++++++------- packages/ui/src/sync/sync-context.tsx | 47 +++++++++++ 5 files changed, 148 insertions(+), 36 deletions(-) diff --git a/packages/ui/src/components/chat/ChatContainer.tsx b/packages/ui/src/components/chat/ChatContainer.tsx index 4c17a00b..33fe11a0 100644 --- a/packages/ui/src/components/chat/ChatContainer.tsx +++ b/packages/ui/src/components/chat/ChatContainer.tsx @@ -326,7 +326,7 @@ export const ChatContainer: React.FC = () => { return flattenBlockingRequests(questionsMap, scopedSessionIds); }, [questionsMap, scopedSessionIds]); const sessionIsWorking = React.useMemo(() => { - if (!currentSessionId || sessionPermissions.length > 0) { + if (!currentSessionId || sessionPermissions.length > 0 || sessionQuestions.length > 0) { return false; } @@ -345,7 +345,7 @@ export const ChatContainer: React.FC = () => { && lastMessage.role === 'assistant' && typeof (lastMessage as { time?: { completed?: number } }).time?.completed !== 'number', ); - }, [activeStreamingPhase, currentSessionId, sessionMessages, sessionPermissions.length, sessionStatusForCurrent.type, streamingMessageId]); + }, [activeStreamingPhase, currentSessionId, sessionMessages, sessionPermissions.length, sessionQuestions.length, sessionStatusForCurrent.type, streamingMessageId]); const activeRetryStatus = React.useMemo(() => { if (!currentSessionId || sessionStatusForCurrent.type !== 'retry') { return null; diff --git a/packages/ui/src/components/chat/message/parts/ToolPart.tsx b/packages/ui/src/components/chat/message/parts/ToolPart.tsx index 0ce75da7..35219d3b 100644 --- a/packages/ui/src/components/chat/message/parts/ToolPart.tsx +++ b/packages/ui/src/components/chat/message/parts/ToolPart.tsx @@ -1526,7 +1526,7 @@ const ToolExpandedContent: React.FC = React.memo(({ ); }; - // Question tool: show parsed Q&A summary + // Question tool: show parsed Q&A summary or question content from input if (part.tool === 'question') { if (state.status === 'completed' && hasStringOutput) { const parsedQA = parseQuestionOutput(outputString); @@ -1560,6 +1560,35 @@ const ToolExpandedContent: React.FC = React.memo(({ ); } + // Show question content from input whenever available, whether the tool is + // pending/running or completed without parseable output. This ensures question + // text persists across refreshes even if the QuestionCard store data is lost. + const questionInput = input as { questions?: Array<{ question?: string; header?: string; options?: Array<{ label: string; description: string }>; multiple?: boolean }> } | undefined; + if (questionInput?.questions && Array.isArray(questionInput.questions) && questionInput.questions.length > 0) { + return renderScrollableBlock( +
+ {questionInput.questions.map((q, index) => ( +
+ {q.header ? ( +
{q.header}
+ ) : null} +
{q.question}
+ {Array.isArray(q.options) && q.options.length > 0 ? ( +
+ {q.options.map((opt) => ( + + {opt.label} + + ))} +
+ ) : null} +
+ ))} +
, + { maxHeightClass: 'max-h-[40vh]' } + ); + } + return
Awaiting response...
; } diff --git a/packages/ui/src/hooks/useAssistantStatus.ts b/packages/ui/src/hooks/useAssistantStatus.ts index 53234568..3926f305 100644 --- a/packages/ui/src/hooks/useAssistantStatus.ts +++ b/packages/ui/src/hooks/useAssistantStatus.ts @@ -3,7 +3,7 @@ import type { AssistantMessage, Message, Part, ReasoningPart, TextPart, ToolPart import type { MessageStreamPhase } from '@/stores/types/sessionTypes'; import { useSessionUIStore } from '@/sync/session-ui-store'; -import { useDirectorySync, useSessionPermissions, useSessionStatus } from '@/sync/sync-context'; +import { useDirectorySync, useSessionPermissions, useSessionQuestions, useSessionStatus } from '@/sync/sync-context'; import { isFullySyntheticMessage } from '@/lib/messages/synthetic'; import { useCurrentSessionActivity } from './useSessionActivity'; @@ -163,6 +163,7 @@ export function useAssistantStatus(): AssistantStatusSnapshot { ); const sessionPermissionRequests = useSessionPermissions(currentSessionId ?? ''); + const sessionQuestionRequests = useSessionQuestions(currentSessionId ?? ''); const sessionAbortRecord = useSessionUIStore( React.useCallback((state) => { @@ -427,11 +428,26 @@ export function useAssistantStatus(): AssistantStatusSnapshot { } const hasPendingPermission = sessionPermissionRequests.length > 0; + const hasPendingQuestion = sessionQuestionRequests.length > 0; - if (!hasPendingPermission) { + if (!hasPendingPermission && !hasPendingQuestion) { return baseWorking; } + if (hasPendingQuestion) { + return { + ...baseWorking, + statusText: null, + isWorking: false, + hasWorkingContext: false, + hasActiveTools: false, + canAbort: false, + activePartType: undefined, + activeToolName: undefined, + retryInfo: null, + }; + } + return { ...baseWorking, statusText: 'waiting for permission', @@ -439,7 +455,7 @@ export function useAssistantStatus(): AssistantStatusSnapshot { canAbort: false, retryInfo: null, }; - }, [baseWorking, sessionPermissionRequests]); + }, [baseWorking, sessionPermissionRequests, sessionQuestionRequests]); return { forming, diff --git a/packages/ui/src/sync/bootstrap.ts b/packages/ui/src/sync/bootstrap.ts index 7ff85bf7..caafc994 100644 --- a/packages/ui/src/sync/bootstrap.ts +++ b/packages/ui/src/sync/bootstrap.ts @@ -4,6 +4,14 @@ import type { GlobalState, State } from "./types" const cmp = (a: string, b: string) => (a < b ? -1 : a > b ? 1 : 0) +const requestSignature = (items: Array<{ id: string }> | undefined): string => { + if (!items || items.length === 0) return "" + return items + .map((item) => item.id) + .sort(cmp) + .join("|") +} + function groupBySession(input: T[]) { return input.reduce>((acc, item) => { if (!item?.id || !item.sessionID) return acc @@ -129,44 +137,56 @@ export async function bootstrapDirectory(input: { set({ vcs: x.data ?? current.vcs }) }), ), - retry(() => - sdk.permission.list().then((x) => { - const grouped = groupBySession( - (x.data ?? []).filter((perm): perm is PermissionRequest => !!perm?.id && !!perm.sessionID), - ) - const permission: Record = {} - // Clear sessions no longer having permissions - const current = getState() - for (const sessionID of Object.keys(current.permission ?? {})) { - if (!grouped[sessionID]) permission[sessionID] = [] - } - // Set grouped permissions sorted by id - for (const [sessionID, perms] of Object.entries(grouped)) { - permission[sessionID] = perms - .filter((p) => !!p?.id) - .sort((a, b) => cmp(a.id, b.id)) - } - set({ permission }) - }), - ), - retry(() => - sdk.question.list().then((x) => { + retry(async () => { + const before = getState() + const beforeSignatures = new Map( + Object.entries(before.question ?? {}).map(([sessionID, questions]) => [sessionID, requestSignature(questions)]), + ) + const x = await sdk.question.list(directory ? { directory } : undefined) const grouped = groupBySession( (x.data ?? []).filter((q): q is QuestionRequest => !!q?.id && !!q.sessionID), ) - const question: Record = {} const current = getState() - for (const sessionID of Object.keys(current.question ?? {})) { - if (!grouped[sessionID]) question[sessionID] = [] - } + const merged = { ...current.question } for (const [sessionID, questions] of Object.entries(grouped)) { - question[sessionID] = questions + merged[sessionID] = questions .filter((q) => !!q?.id) .sort((a, b) => cmp(a.id, b.id)) } - set({ question }) - }), - ), + for (const sessionID of beforeSignatures.keys()) { + if (grouped[sessionID]) continue + const beforeSignature = beforeSignatures.get(sessionID) ?? "" + const currentSignature = requestSignature(current.question[sessionID]) + if (currentSignature !== beforeSignature) continue + delete merged[sessionID] + } + set({ question: merged }) + }), + retry(async () => { + const before = getState() + const beforeSignatures = new Map( + Object.entries(before.permission ?? {}).map(([sessionID, permissions]) => [sessionID, requestSignature(permissions)]), + ) + const x = await sdk.permission.list(directory ? { directory } : undefined) + const grouped = groupBySession( + (x.data ?? []).filter((perm): perm is PermissionRequest => !!perm?.id && !!perm?.sessionID), + ) + const current = getState() + const merged = { ...current.permission } + for (const [sessionID, perms] of Object.entries(grouped)) { + merged[sessionID] = perms + .filter((p) => !!p?.id) + .sort((a, b) => cmp(a.id, b.id)) + } + for (const sessionID of beforeSignatures.keys()) { + if (grouped[sessionID]) continue + const beforeSignature = beforeSignatures.get(sessionID) ?? "" + const currentSignature = requestSignature(current.permission[sessionID]) + if (currentSignature !== beforeSignature) continue + delete merged[sessionID] + } + set({ permission: merged }) + }), ]) const errors = results diff --git a/packages/ui/src/sync/sync-context.tsx b/packages/ui/src/sync/sync-context.tsx index 44897deb..36d20a7d 100644 --- a/packages/ui/src/sync/sync-context.tsx +++ b/packages/ui/src/sync/sync-context.tsx @@ -98,6 +98,13 @@ let bootedAt = 0 const BOOT_DEBOUNCE_MS = 1500 const RECONNECT_MESSAGE_LIMIT = 200 const RECONNECT_SKIP_PARTS = new Set(["patch", "step-start", "step-finish"]) +const requestSignature = (items: Array<{ id: string }> | undefined): string => { + if (!items || items.length === 0) return "" + return items + .map((item) => item.id) + .sort(cmp) + .join("|") +} const cmp = (a: string, b: string) => (a < b ? -1 : a > b ? 1 : 0) @@ -719,6 +726,46 @@ async function resyncDirectoryAfterReconnect( setIndexedSessionMessages(routingIndex, sessionId, directory, nextMessages) })) + // Re-fetch pending questions on reconnect — they may have been asked + // during the SSE disconnection window and will not arrive via SSE events. + // Overwrite sessions covered by API response, and clear reconnect candidates + // that remain unchanged during the request but are absent from the response. + // If SSE changed a session while the request was in-flight, keep that data. + try { + const before = store.getState() + const beforeSignatures = new Map( + candidateSessionIds.map((sessionId) => [sessionId, requestSignature(before.question[sessionId])]), + ) + const pendingQuestions = await opencodeClient.listPendingQuestions({ directories: [directory] }) + const grouped: Record = {} + for (const q of pendingQuestions) { + if (!q?.id || !q.sessionID) continue + const list = grouped[q.sessionID] + if (list) list.push(q) + else grouped[q.sessionID] = [q] + } + // Sort each group by id for binary-search compatibility + for (const sessionId of Object.keys(grouped)) { + grouped[sessionId].sort((a, b) => (a.id < b.id ? -1 : a.id > b.id ? 1 : 0)) + } + store.setState((state: DirectoryStore) => { + const merged = { ...state.question } + for (const [sessionId, questions] of Object.entries(grouped)) { + merged[sessionId] = questions + } + for (const sessionId of candidateSessionIds) { + if (grouped[sessionId]) continue + const beforeSignature = beforeSignatures.get(sessionId) ?? "" + const currentSignature = requestSignature(state.question[sessionId]) + if (currentSignature !== beforeSignature) continue + delete merged[sessionId] + } + return { question: merged } + }) + } catch { + // Non-fatal: question resync best-effort + } + ingestDirectoryStateIntoRoutingIndex(routingIndex, directory, store.getState()) }